diff --git a/README.md b/README.md index 4324c772..8865045c 100644 --- a/README.md +++ b/README.md @@ -189,65 +189,56 @@ on — and removes the ones you select from those tools. It needs no Databricks ### Skills (optional) -Configure Unity Catalog Skills for your coding tools with `ucode configure skills`: +Use `ucode skill add` to download Unity Catalog Skills or expose them through the skills MCP: ```bash -# Utility tools only: register the schema-less skills MCP connection, no download. -ucode configure skills +# Browse finalized skills across the metastore and choose which ones to download. +ucode skill add # Download mode: fetch every skill in the schema to disk (and register the connection). -ucode configure skills --location main.default --path /abs/project/dir +ucode skill add --location main.default --path /abs/project/dir -# Download a named subset of the schema's skills instead of all of them. -ucode configure skills --location main.default --skill my-skill +# Download named skills by bare name or fully-qualified name. +ucode skill add --location main.default --skills my-skill,other-skill +ucode skill add --skills main.default.my-skill,main.default.other-skill + +# Add schemas to every configured agent's MCP scope, keeping existing schemas. +ucode skill add --location main.default,ml.prod --mcp -# MCP mode: expose the schema's skills as MCP tools instead of downloading. -ucode configure skills --location main.default,ml.prod --mcp +# Add a schema only to selected agents, setting them up first if needed. +ucode skill add --location main.default --mcp --agents claude,codex ``` -- **Bare command** (no `--location`) registers the schema-less skills MCP connection — the - cross-schema utility tools only — and downloads nothing. `--mcp` with no `--location` does the - same. - **Download mode** (with `--location`, no `--mcp`) writes each skill flat as `/SKILL.md` (plus its bundled files) into both `.claude/skills/` and `.agents/skills/`. `--path` (an existing absolute project directory) is optional; when omitted, skills are written to user-level skill - directories. Any pre-existing skill dir prompts before it's overwritten. It then registers a - schema-less skills MCP connection, leaving any prior `--mcp` scope untouched. - `--skill [,…]` narrows the download to the named skills (by leaf name) from the schema - instead of all of them; requested names not found in the schema warn and are skipped. `--skill` - requires a single `--location`, is download-only, and is rejected with `--mcp`. -- **MCP mode** (`--location … --mcp`) sets the connection's location set to exactly `` - (override-only) and rebuilds its `?schema=` URL; no files are downloaded and `--path` is rejected. + directories. Any pre-existing skill dir prompts before it's overwritten. `--skills` narrows the + download to named securables. With neither `--location` nor `--skills`, a searchable picker lists + the finalized skills visible in the metastore. Bare names require one `--location`; + fully-qualified names derive it. Download mode always writes both directory families, so + `--agents` is accepted only with `--mcp`. +- **MCP mode** (`--location … --mcp`) unions schemas into the selected agents' existing scopes. + Without `--agents`, it updates every configured agent. No files are downloaded. Each run prints the registered server, its URL, the configured agents, and its tools, and reminds you to run `ucode ` (existing agent sessions need a restart before the MCP tools load). -#### Add skill scopes without replacing existing ones +#### Remove skill MCP scopes -`ucode skill add` registers skills additively, keeping anything already configured. With `--mcp` it -adds the schemas to every configured agent's scope, or only to `--agents` when supplied. Agents that -are not configured yet are set up first. Without `--mcp`, it downloads skills to disk; download mode -always writes both directory families and does not accept `--agents`. `--skills` narrows a download -to a subset of one schema's skills. With no selection flags, a searchable picker lists finalized -skills visible in the metastore. +`ucode skill remove --mcp` interactively lists developer-configured schemas and the agents using +them. Selected schemas are removed from every agent, or only from `--agents` when supplied. The +schema-less utility connection remains registered after its last schema is removed. Downloaded +skill removal is not yet supported. ```bash -# Browse the metastore and choose skills to download. -ucode skill add - -# Add schemas to the skills MCP scope, keeping any already configured. -ucode skill add --location main.default,ml.prod --mcp - -# Add a schema only to selected agents. -ucode skill add --location main.default --mcp --agents claude,codex +ucode skill remove --mcp +ucode skill remove --mcp --agents codex +``` -# Download a schema's skills to disk, keeping existing downloads. -ucode skill add --location main.default +`ucode configure skills` is deprecated but remains available for existing scripts. Its MCP mode +keeps its original replacement semantics, and its bare form remains the way to register only the +schema-less utility connection during the transition. -# Download a named subset, by bare name (with --location) or fully-qualified name. -ucode skill add --location main.default --skills my-skill,other-skill -ucode skill add --skills main.default.my-skill,main.default.other-skill -``` ### Managed config for a workspace (admins) @@ -379,15 +370,13 @@ The output looks like: | `ucode mcp add --agents claude --services system.ai.slack` | Set up the agent(s) if needed and register the server for them | | `ucode mcp remove` | Interactively unregister configured MCP servers from your coding tools | | `ucode mcp remove --agents codex` | Unregister selected servers from specific agents only | -| `ucode configure skills` | Register the skills MCP connection (utility tools only); no skills download | -| `ucode configure skills --location main.default [--path ]` | Download a schema's skills to disk (under ``, or your home dir) and register a schema-less skills MCP connection | -| `ucode configure skills --location main.default --skill my-skill` | Download only the named skill(s) from a schema (comma-separated for several) | -| `ucode configure skills --location main.default --mcp` | Expose a schema's skills as MCP tools (override-only) instead of downloading | -| `ucode skill add` | Interactively choose finalized metastore skills to download | | `ucode skill add --location main.default --mcp` | Add schemas to the skills MCP scope, keeping any already configured (additive; never replaces) | | `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` | Interactively choose finalized metastore skills to download | | `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 [--agents codex]` | Interactively remove developer schemas from all or selected agent MCP scopes | +| `ucode configure skills ...` | Deprecated compatibility command; existing behavior is preserved | | `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 ab8ea506..b093de1c 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -105,6 +105,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 @@ -1473,6 +1472,48 @@ 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, + agents: Annotated[ + str | None, + typer.Option( + "--agents", + help="Comma-separated coding agents to remove selected schemas from.", + ), + ] = None, +) -> None: + """Remove Databricks Skill schemas from coding tools. + + MCP removal is interactive and can be scoped to specific configured agents. + Removing downloaded skills is not supported yet. + """ + try: + if not mcp: + raise RuntimeError( + "Removing downloaded skills is not supported yet. Pass --mcp to remove " + "schemas from the skills MCP connection." + ) + requested_agents = ( + None + if agents is None + else ({agent.strip().lower() for agent in agents.split(",") if agent.strip()} or None) + ) + remove_skills_command(agents=requested_agents) + 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[ @@ -3085,7 +3126,7 @@ def configure_mcp( raise typer.Exit(130) from None -@configure_app.command("skills") +@configure_app.command("skills", deprecated=True) def configure_skills( location: Annotated[ str | None, @@ -3124,6 +3165,10 @@ def configure_skills( download to a named subset of a single schema's skills (requires exactly one ``--location``). """ + print_warning( + "`ucode configure skills` is deprecated. Use `ucode skill add` to download " + "skills or add MCP scopes, and `ucode skill remove --mcp` to remove MCP scopes." + ) try: locations = _parse_skill_locations(location) # `--skill` absent -> None (whole schema); present (even empty) -> the diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 1f1a49ce..41696423 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2382,3 +2382,109 @@ 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: + 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(agents: set[str] | None = None) -> int: + """Interactively remove developer skill schemas from selected agents' MCP scopes.""" + state = load_state() + workspace, profile, clients = setup_mcp_clients( + state, + "Remove Skills MCP", + require_auth=False, + action_note="Removing from", + agents=agents, + ) + 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 + } + 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) + default = _skill_mcp_locations(state) + overrides = _skill_location_overrides(entry) + if agents is None: + new_default = [location for location in default if location not in remove_locations] + overrides = { + client: [location for location in client_locations if location not in remove_locations] + for client, client_locations in overrides.items() + } + else: + new_default = default + for client in clients: + _set_skill_location_override( + overrides, + client, + [ + location + for location in skill_locations_for_client(entry, client) + if location not in remove_locations + ], + default, + ) + + _update_skills_mcp( + state, + workspace, + profile, + clients, + new_default, + location_overrides=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 f44e8d2c..88dfcbea 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1213,6 +1213,33 @@ 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_forwards_agent_scope(self): + with patch("ucode.cli.remove_skills_command") as remove: + result = runner.invoke(app, ["skill", "remove", "--mcp", "--agents", "claude, codex"]) + + assert result.exit_code == 0, result.output + remove.assert_called_once_with(agents={"claude", "codex"}) + + +class TestConfigureSkillsDeprecation: + def test_warns_and_keeps_legacy_dispatch(self): + with patch("ucode.cli.configure_skills_mcp_command") as configure: + result = runner.invoke(app, ["configure", "skills", "--location", "a.b", "--mcp"]) + + assert result.exit_code == 0, result.output + assert "deprecated" in _strip_ansi(result.output).lower() + configure.assert_called_once_with(["a.b"]) + + 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 e604c9c5..b4d581a6 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2577,6 +2577,70 @@ def test_agents_updates_only_selected_client_scope(self, monkeypatch): assert configured == [("claude", f"{WS}/ai-gateway/skills/?schema=A.a&schema=B.b")] +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_agent_scope_removes_from_only_selected_client(self, monkeypatch): + state = self._state() + configured = self._stub(monkeypatch, state, ["A.a"]) + + assert mcp.remove_skills_command(agents={"claude"}) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert entry["skill_locations"] == ["A.a", "B.b"] + assert mcp.skill_locations_for_client(entry, "claude") == ["B.b"] + assert mcp.skill_locations_for_client(entry, "codex") == ["A.a", "B.b"] + assert configured == [("claude", f"{WS}/ai-gateway/skills/?schema=B.b")] + + 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_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] = []