diff --git a/README.md b/README.md index 814f5ff1..6c37679a 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,9 @@ Unity Catalog Skills can be registered as MCP tools or downloaded into local agent skill directories. ```bash +# Set up the Databricks skills MCP so your agents can create and manage skills through ug. +ug skills + # List configured skills and how each was configured. ug skills list @@ -130,6 +133,7 @@ ug skills remove --names main.default.my-skill | `ug mcp add` | Add MCP servers without removing existing registrations | | `ug mcp remove` | Unregister configured MCP servers | | `ug mcp list` | List configured MCP servers and connection status | +| `ug skills` | Set up the Databricks skills MCP so agents can create and manage skills | | `ug skills list` | List configured skills and how each was configured | | `ug skills add` | Add skill MCP scopes or download skills | | `ug skills remove` | Remove skill MCP scopes or downloaded skills | diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 01012957..836a5ddd 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -106,6 +106,7 @@ add_mcp_command, add_skills_command, available_mcp_clients, + configure_bare_skills_mcp_command, configure_mcp_command, configure_skills_mcp_command, configure_skills_mcp_picker_command, @@ -1385,7 +1386,7 @@ def format_help(self, ctx: _click.Context, formatter: _click.HelpFormatter) -> N help="Inspect and manage the Databricks MCP servers ug configures for your coding agents.", rich_help_panel="Tools and Skills", ) -skill_app = typer.Typer(add_completion=False, no_args_is_help=True) +skill_app = typer.Typer(add_completion=False, no_args_is_help=False) app.add_typer( skill_app, name="skills", @@ -1572,6 +1573,32 @@ def _stdin_is_interactive() -> bool: return sys.stdin.isatty() +@skill_app.callback(invoke_without_command=True) +def skills(ctx: typer.Context) -> None: + """Databricks Skills for your coding tools. + + With no subcommand, prints this help, then registers the skills MCP connection + (utility tools only) for your configured agents, keeping any existing scope. + """ + if ctx.invoked_subcommand is not None: + return + console.print(ctx.get_help()) + try: + install_databricks_cli(minimum=SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION) + first_time = configure_bare_skills_mcp_command() + except (RuntimeError, ValueError) as exc: + print_err(str(exc)) + raise typer.Exit(1) from None + except KeyboardInterrupt: + print_err("Interrupted.") + raise typer.Exit(130) from None + if first_time: + print_note( + "To create a skill, ask your agent to create one with the Databricks skills " + "registry MCP, which registers it in Unity Catalog." + ) + + @skill_app.command("list") def skills_list() -> None: """List the skills configured for your coding tools and how each was configured.""" diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 7d0ef16d..da0314ad 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -1390,6 +1390,7 @@ def setup_mcp_clients( require_auth: bool = True, action_note: str = "Configuring for", agents: set[str] | None = None, + quiet: bool = False, ) -> tuple[str, str | None, list[str]]: """Validate the workspace, resolve configured MCP clients, and prepare auth. @@ -1404,6 +1405,9 @@ def setup_mcp_clients( the configured MCP clients, so the operation touches only those agents instead of every configured one. Requested agents that aren't configured/installed raise a clear error. + + ``quiet`` suppresses the section header and the ``action_note`` line so a repeat, + no-op registration prints nothing; the missing-client warnings are kept. """ workspace = state.get("workspace") if not workspace: @@ -1441,9 +1445,10 @@ def setup_mcp_clients( apply_pat_environment(state) ensure_databricks_auth(workspace, profile) - print_section(section) - client_names = ", ".join(str(MCP_CLIENTS[client]["display"]) for client in clients) - print_note(f"{action_note}: {client_names}") + if not quiet: + print_section(section) + client_names = ", ".join(str(MCP_CLIENTS[client]["display"]) for client in clients) + print_note(f"{action_note}: {client_names}") for client in missing_clients: print_warning( f"{MCP_CLIENTS[client]['display']} is configured in ucode but not installed; " @@ -2475,15 +2480,46 @@ def _skill_mcp_locations(state: dict) -> list[str]: def register_schemaless_skills_connection( - state: dict, workspace: str, profile: str | None, clients: list[str] + state: dict, + workspace: str, + profile: str | None, + clients: list[str], + *, + print_summary: bool = True, ) -> None: """Register/keep the skills MCP connection without changing its schema set. Download mode calls this after writing files: it preserves each client's prior - ``--mcp`` scope and otherwise registers the bare schema-less route (utility tools only).""" + ``--mcp`` scope and otherwise registers the bare schema-less route (utility tools only). + Downloads pass ``print_summary=False`` so the connection summary never buries any + per-skill download failures the download step already reported.""" _update_skills_mcp( - state, workspace, profile, clients, _skill_locations_by_client_from_state(state) + state, + workspace, + profile, + clients, + _skill_locations_by_client_from_state(state), + print_summary=print_summary, + ) + + +def configure_bare_skills_mcp_command() -> bool: + """Register the schema-less skills MCP connection for every configured agent. + + The simple entrypoint behind a bare ``ug skills`` (replacing ``ug configure skills`` + with no arguments). Re-registers on every run, preserving any client's existing + ``--mcp`` scope, but only the first run prints anything (the setup header and the + connection summary) -- a repeat ``ug skills`` re-registers silently. Returns whether + no skills connection existed beforehand, so the caller can also show first-run + guidance only then. + """ + state = load_state() + first_time = _skills_entry(list(state.get("mcp_servers") or [])) is None + workspace, profile, clients = setup_mcp_clients(state, "Skills", quiet=not first_time) + register_schemaless_skills_connection( + state, workspace, profile, clients, print_summary=first_time ) + return first_time def _union_locations(base: list[str], new: list[str]) -> list[str]: diff --git a/src/ucode/skills_download.py b/src/ucode/skills_download.py index 8dac9cc5..f95a72fb 100644 --- a/src/ucode/skills_download.py +++ b/src/ucode/skills_download.py @@ -420,12 +420,12 @@ def configure_location_skills_download_command(locations: list[str], *, path: st ``--mcp`` set survives a download run. Downloading a named subset instead of whole schemas is a separate command (``configure_selected_skills_download_command``).""" state = load_state() - workspace, profile, clients = setup_mcp_clients(state, "Skills") + workspace, profile, clients = setup_mcp_clients(state, "Skills", quiet=True) token = get_databricks_token(workspace, profile) download_skills_from_schema_locations(workspace, token, locations, path) - register_schemaless_skills_connection(state, workspace, profile, clients) + register_schemaless_skills_connection(state, workspace, profile, clients, print_summary=False) return 0 @@ -436,12 +436,12 @@ def configure_selected_skills_download_command(fqns: list[str], path: str | None ``download_selected_skills`` (which alone does not register), then registers/keeps the schema-less MCP connection, exactly as the whole-schema download does.""" state = load_state() - workspace, profile, clients = setup_mcp_clients(state, "Skills") + workspace, profile, clients = setup_mcp_clients(state, "Skills", quiet=True) token = get_databricks_token(workspace, profile) download_selected_skills(workspace, token, fqns, path) - register_schemaless_skills_connection(state, workspace, profile, clients) + register_schemaless_skills_connection(state, workspace, profile, clients, print_summary=False) return 0 @@ -504,7 +504,7 @@ def configure_skills_download_picker_command(path: str | None = None) -> int: Ctrl-C downloads nothing and leaves the connection untouched. """ state = load_state() - workspace, profile, clients = setup_mcp_clients(state, "Skills") + workspace, profile, clients = setup_mcp_clients(state, "Skills", quiet=True) token = get_databricks_token(workspace, profile) roots = skill_dir_roots(path) @@ -514,7 +514,7 @@ def configure_skills_download_picker_command(path: str | None = None) -> int: return 0 download_selected_skills(workspace, token, fqns, path) - register_schemaless_skills_connection(state, workspace, profile, clients) + register_schemaless_skills_connection(state, workspace, profile, clients, print_summary=False) return 0 diff --git a/tests/test_cli.py b/tests/test_cli.py index 601871a3..6a20f55d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2018,6 +2018,51 @@ def test_path_without_location_exit_1(self): mock_download.assert_not_called() +class TestSkillsEntrypoint: + """Bare `ug skills` registers the schema-less MCP connection, then prints help.""" + + @pytest.fixture(autouse=True) + def _stub_install_cli(self): + with patch("ucode.cli.install_databricks_cli") as mock_install: + yield mock_install + + def test_first_run_configures_shows_help_and_create_note(self, _stub_install_cli): + from ucode.databricks import SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION + + with patch("ucode.cli.configure_bare_skills_mcp_command", return_value=True) as mock_conf: + result = runner.invoke(app, ["skills"]) + + assert result.exit_code == 0, result.output + mock_conf.assert_called_once_with() + _stub_install_cli.assert_called_once_with(minimum=SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION) + output = _strip_ansi(result.output) + assert "To create a skill" in output + # Still prints the group help it always showed, and prints it before the MCP messages. + assert "Usage:" in output + assert output.index("Usage:") < output.index("To create a skill") + for command in ("list", "add", "remove"): + assert command in output + + def test_already_configured_omits_create_note(self): + with patch("ucode.cli.configure_bare_skills_mcp_command", return_value=False): + result = runner.invoke(app, ["skills"]) + + assert result.exit_code == 0, result.output + output = _strip_ansi(result.output) + assert "To create a skill" not in output + assert "Usage:" in output + + def test_subcommand_skips_entrypoint_configuration(self): + with ( + patch("ucode.cli.configure_bare_skills_mcp_command") as mock_conf, + patch("ucode.cli.list_configured_skills_command"), + ): + result = runner.invoke(app, ["skills", "list"]) + + assert result.exit_code == 0, result.output + mock_conf.assert_not_called() + + class TestSkillsAddCommand: """`ucode skills add` is the additive sibling of `configure skills`: `--mcp` unions schemas into the connection scope, the default mode downloads.""" diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 63b978db..2845ffab 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2631,6 +2631,51 @@ def test_preserves_prior_mcp_location_set(self, monkeypatch): assert _find_skills(state["mcp_servers"])[0]["skill_locations"] == ["X.x", "Y.y"] + def test_download_path_suppresses_summary(self, monkeypatch, capsys): + self._stub(monkeypatch) + state = _skills_state([]) + + mcp.register_schemaless_skills_connection(state, WS, None, ["claude"], print_summary=False) + + assert _unwrap(capsys.readouterr().out) == "" + + +class TestConfigureBareSkillsMcpCommand: + def _stub(self, monkeypatch, state): + _stub_location_base(monkeypatch, state) + saved_states: list[dict] = [] + configured: list[str] = [] + monkeypatch.setattr( + mcp, "configure_client_mcp_server", lambda client, *a, **kw: configured.append(client) + ) + monkeypatch.setattr(mcp, "save_state", lambda state: saved_states.append(state.copy())) + return saved_states, configured + + def test_first_run_registers_bare_route_and_reports_first_time(self, monkeypatch, capsys): + saved_states, configured = self._stub(monkeypatch, _skills_state()) + + assert mcp.configure_bare_skills_mcp_command() is True + + skills = _find_skills(saved_states[-1]["mcp_servers"]) + assert len(skills) == 1 + assert skills[0]["skill_locations"] == [] + assert configured == ["claude"] + # Only the first run prints: the setup header plus the connection summary. + out = _unwrap(capsys.readouterr().out) + assert "Configuring for: Claude Code" in out + assert "Skills MCP registered" in out + + def test_existing_connection_reregisters_and_reports_not_first_time(self, monkeypatch, capsys): + prior = mcp._resolve_skills_mcp_servers(WS, ["claude"], _by_client(["claude"], []), []) + _, configured = self._stub(monkeypatch, _skills_state(prior)) + + assert mcp.configure_bare_skills_mcp_command() is False + + # A repeat run re-registers silently: no client re-touched, and no output at all + # (neither the setup header nor the connection summary). + assert configured == [] + assert _unwrap(capsys.readouterr().out) == "" + class TestSkillsToolsDescription: def test_bare_route_names_utility_tools_only(self): diff --git a/tests/test_skills_download.py b/tests/test_skills_download.py index 01518401..f2c197cb 100644 --- a/tests/test_skills_download.py +++ b/tests/test_skills_download.py @@ -721,7 +721,11 @@ def _stub(self, monkeypatch): calls: dict[str, object] = {} monkeypatch.setattr(sd, "load_state", lambda: {"state": True}) monkeypatch.setattr( - sd, "setup_mcp_clients", lambda state, section: (WS, "profile", ["claude"]) + sd, + "setup_mcp_clients", + lambda state, section, quiet=False: ( + calls.update(setup_quiet=quiet) or (WS, "profile", ["claude"]) + ), ) monkeypatch.setattr(sd, "get_databricks_token", lambda ws, profile: "token") monkeypatch.setattr( @@ -732,7 +736,9 @@ def _stub(self, monkeypatch): monkeypatch.setattr( sd, "register_schemaless_skills_connection", - lambda state, ws, profile, clients: calls.update(register=(ws, profile, clients)), + lambda state, ws, profile, clients, print_summary=True: calls.update( + register=(ws, profile, clients), print_summary=print_summary + ), ) return calls @@ -743,6 +749,9 @@ def test_downloads_then_registers_connection(self, monkeypatch): assert calls["download"] == (WS, "token", ["a.b"], "/tmp/skills") assert calls["register"] == (WS, "profile", ["claude"]) + # Downloads suppress the connection summary so it can't bury per-skill failures. + assert calls["print_summary"] is False + assert calls["setup_quiet"] is True def test_none_path_threads_through(self, monkeypatch): calls = self._stub(monkeypatch) @@ -758,7 +767,11 @@ def _stub(self, monkeypatch): calls: dict[str, object] = {} monkeypatch.setattr(sd, "load_state", lambda: {"state": True}) monkeypatch.setattr( - sd, "setup_mcp_clients", lambda state, section: (WS, "profile", ["claude"]) + sd, + "setup_mcp_clients", + lambda state, section, quiet=False: ( + calls.update(setup_quiet=quiet) or (WS, "profile", ["claude"]) + ), ) monkeypatch.setattr(sd, "get_databricks_token", lambda ws, profile: "token") monkeypatch.setattr( @@ -769,7 +782,9 @@ def _stub(self, monkeypatch): monkeypatch.setattr( sd, "register_schemaless_skills_connection", - lambda state, ws, profile, clients: calls.update(register=(ws, profile, clients)), + lambda state, ws, profile, clients, print_summary=True: calls.update( + register=(ws, profile, clients), print_summary=print_summary + ), ) return calls @@ -781,6 +796,8 @@ def test_downloads_selected_then_registers(self, monkeypatch): assert calls["download"] == (WS, "token", fqns, "/tmp/skills") assert calls["register"] == (WS, "profile", ["claude"]) + assert calls["print_summary"] is False + assert calls["setup_quiet"] is True def test_none_path_threads_through(self, monkeypatch): calls = self._stub(monkeypatch) @@ -888,7 +905,11 @@ def _stub(self, monkeypatch, fqns): calls: dict[str, object] = {} monkeypatch.setattr(sd, "load_state", lambda: {"state": True}) monkeypatch.setattr( - sd, "setup_mcp_clients", lambda state, section: (WS, "profile", ["claude"]) + sd, + "setup_mcp_clients", + lambda state, section, quiet=False: ( + calls.update(setup_quiet=quiet) or (WS, "profile", ["claude"]) + ), ) monkeypatch.setattr(sd, "get_databricks_token", lambda ws, profile: "token") monkeypatch.setattr( @@ -903,7 +924,9 @@ def _stub(self, monkeypatch, fqns): monkeypatch.setattr( sd, "register_schemaless_skills_connection", - lambda state, ws, profile, clients: calls.update(register=(ws, profile, clients)), + lambda state, ws, profile, clients, print_summary=True: calls.update( + register=(ws, profile, clients), print_summary=print_summary + ), ) return calls @@ -914,6 +937,8 @@ def test_downloads_selected_then_registers(self, tmp_path, monkeypatch): assert calls["download"] == (WS, "token", ["main.default.triage"], str(tmp_path)) assert calls["register"] == (WS, "profile", ["claude"]) + assert calls["print_summary"] is False + assert calls["setup_quiet"] is True def test_cancel_downloads_nothing_and_skips_register(self, monkeypatch): calls = self._stub(monkeypatch, None)