Skip to content

Remove the deprecated ug configure skills command #732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xsh310 merged 1 commit into from
Sep 18, 2026
+13 −325
Merged

Remove the deprecated ug configure skills command #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 0 additions & 77 deletions src/ucode/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@
available_mcp_clients,
configure_bare_skills_mcp_command,
configure_mcp_command,
configure_skills_mcp_command,
configure_skills_mcp_picker_command,
configured_mcp_clients,
list_mcp_command,
Expand Down Expand Up @@ -3588,82 +3587,6 @@ def configure(
raise typer.Exit(130) from None


@configure_app.command("skills")
def configure_skills(
location: Annotated[
str | None,
typer.Option("--location", help="Comma-separated `<catalog>.<schema>` skill scopes."),
] = None,
mcp: Annotated[
bool,
typer.Option("--mcp", help="Mutate the skills MCP connection instead of downloading."),
] = False,
path: Annotated[
str | None,
typer.Option(
"--path",
help="(download) Existing absolute dir to download into; defaults to your home dir.",
),
] = None,
names: Annotated[
str | None,
typer.Option(
"--names",
help="(download) Download exactly these comma-separated fully-qualified "
"`<catalog>.<schema>.<name>` skills, spanning any number of schemas. Not valid "
"with --mcp or --location.",
),
] = None,
) -> None:
"""Configure Databricks Skills for your coding tools.

When ``--location`` is not provided, registers the skills MCP connection with
utility tools only.

When ``--location`` is provided: with ``--mcp``, sets the connection's scope to
exactly the listed schemas (no download); otherwise, downloads every skill in
each schema to disk (under ``--path``, or your home dir when omitted) and
registers the MCP connection with utility tools only. ``--names`` instead
downloads a named set of fully-qualified skills that may span schemas (and takes
no ``--location``).
"""
try:
install_databricks_cli(minimum=SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION)
# `--names` absent -> None (whole schemas via --location); present (even
# empty) -> the explicit FQN set, so `--names ""` downloads nothing.
selected_skills = (
None if names is None else {s.strip() for s in names.split(",") if s.strip()}
)
if mcp and path is not None:
raise RuntimeError("--path is not valid with --mcp.")
if mcp and selected_skills is not None:
raise RuntimeError("--names is not valid with --mcp; it only applies when downloading.")
if selected_skills is not None and location is not None:
raise RuntimeError("--names takes fully-qualified names; drop --location.")
if selected_skills is not None:
invalid = sorted(s for s in selected_skills if not _is_qualified_skill_name(s))
if invalid:
raise RuntimeError(
"--names entries must be fully-qualified `<catalog>.<schema>.<name>` names "
f"(invalid: {', '.join(invalid)})."
)
configure_selected_skills_download_command(sorted(selected_skills), path)
return
locations = _parse_skill_locations(location)
if path is not None and not locations:
raise RuntimeError("--path only applies when downloading with --location.")
if mcp or not locations:
configure_skills_mcp_command(locations)
else:
configure_location_skills_download_command(locations, path=path)
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


@app.command("export", rich_help_panel="Manage")
def export_cmd(
file_path: Annotated[
Expand Down
26 changes: 7 additions & 19 deletions src/ucode/mcp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,7 @@ def configure_mcp_command(

excluded_sources = exclude_sources or set()
original_mcp_servers: list[dict] = list(state.get("mcp_servers") or [])
# Skills connections are managed by `configure skills`, so keep them out of
# Skills connections are managed by the `ug skills` commands, so keep them out of
# the picker and carry them through untouched.
skills_servers = _skills_entries(original_mcp_servers)
picker_servers = [s for s in original_mcp_servers if s.get("kind") != SKILLS_MCP_KIND]
Expand Down Expand Up @@ -1782,7 +1782,7 @@ def remove_mcp_command(agents: set[str] | None = None) -> int:
"""`ucode mcp remove`: interactively unregister configured MCP servers.

Shows the servers currently configured (skills connections excluded — they're
owned by `configure skills`) and removes the ones you select. It never adds or
owned by the `ug skills` commands) and removes the ones you select. It never adds or
reconfigures anything, and needs no Databricks auth.

Without ``agents``, a selected server is removed from every coding tool it's
Expand Down Expand Up @@ -2461,17 +2461,6 @@ def _update_skills_mcp(
return changed or original != working


def configure_skills_mcp_command(locations: list[str]) -> int:
"""Set every configured client's skill scope to ``locations``."""
state = load_state()
workspace, profile, clients = setup_mcp_clients(state, "Skills MCP")
locations_by_client = _skill_locations_by_client_from_state(state)
for client in clients:
locations_by_client[client] = list(locations)
_update_skills_mcp(state, workspace, profile, clients, locations_by_client)
return 0


def _skill_mcp_locations(state: dict) -> list[str]:
"""The skills MCP connection's ``skill_locations``, or ``[]`` if none exists."""
entry = _skills_entry(list(state.get("mcp_servers") or []))
Expand Down Expand Up @@ -2506,12 +2495,11 @@ def register_schemaless_skills_connection(
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.
The simple entrypoint behind a bare ``ug skills``. 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
Expand Down
135 changes: 2 additions & 133 deletions tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1887,137 +1887,6 @@ def test_labels_cached_fallback_when_live_auth_fails(self):
assert freshness == "cached"


class TestConfigureSkillsCommand:
@pytest.fixture(autouse=True)
def _stub_install_cli(self):
with patch("ucode.cli.install_databricks_cli") as mock_install:
yield mock_install

def test_requires_skills_mcp_cli_floor(self, _stub_install_cli):
from ucode.databricks import SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION

with patch("ucode.cli.configure_skills_mcp_command"):
result = runner.invoke(app, ["configure", "skills", "--location", "a.b", "--mcp"])
assert result.exit_code == 0, result.output
_stub_install_cli.assert_called_once_with(minimum=SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION)

def test_mcp_flag_dispatches_location_set(self):
with patch("ucode.cli.configure_skills_mcp_command") as mock_mcp:
result = runner.invoke(app, ["configure", "skills", "--location", "a.b", "--mcp"])
assert result.exit_code == 0, result.output
mock_mcp.assert_called_once_with(["a.b"])

def test_comma_location_yields_multiple_schemas(self):
with patch("ucode.cli.configure_skills_mcp_command") as mock_mcp:
result = runner.invoke(app, ["configure", "skills", "--location", "a.b, c.d", "--mcp"])
assert result.exit_code == 0, result.output
mock_mcp.assert_called_once_with(["a.b", "c.d"])

def test_default_mode_dispatches_download_with_path(self):
with patch("ucode.cli.configure_location_skills_download_command") as mock_download:
result = runner.invoke(
app, ["configure", "skills", "--location", "a.b", "--path", "/tmp/skills"]
)
assert result.exit_code == 0, result.output
mock_download.assert_called_once_with(["a.b"], path="/tmp/skills")

def test_default_mode_without_path_dispatches_download(self):
with patch("ucode.cli.configure_location_skills_download_command") as mock_download:
result = runner.invoke(app, ["configure", "skills", "--location", "a.b"])
assert result.exit_code == 0, result.output
mock_download.assert_called_once_with(["a.b"], path=None)

def test_skill_downloads_fully_qualified_across_schemas(self):
with patch("ucode.cli.configure_selected_skills_download_command") as mock_download:
result = runner.invoke(app, ["configure", "skills", "--names", "a.b.s1, c.d.s2"])
assert result.exit_code == 0, result.output
mock_download.assert_called_once_with(["a.b.s1", "c.d.s2"], None)

def test_skill_threads_path_through(self):
with patch("ucode.cli.configure_selected_skills_download_command") as mock_download:
result = runner.invoke(
app, ["configure", "skills", "--names", "a.b.s1", "--path", "/tmp/skills"]
)
assert result.exit_code == 0, result.output
mock_download.assert_called_once_with(["a.b.s1"], "/tmp/skills")

def test_skill_with_location_exit_1(self):
with patch("ucode.cli.configure_selected_skills_download_command") as mock_download:
result = runner.invoke(
app, ["configure", "skills", "--location", "a.b", "--names", "a.b.s1"]
)
assert result.exit_code == 1
assert "--names takes fully-qualified names; drop --location" in _strip_ansi(result.output)
mock_download.assert_not_called()

def test_bare_skill_exit_1(self):
with patch("ucode.cli.configure_selected_skills_download_command") as mock_download:
result = runner.invoke(app, ["configure", "skills", "--names", "my_skill"])
assert result.exit_code == 1
assert "must be fully-qualified" in _strip_ansi(result.output)
mock_download.assert_not_called()

def test_skill_with_mcp_exit_1(self):
with (
patch("ucode.cli.configure_skills_mcp_command") as mock_mcp,
patch("ucode.cli.configure_selected_skills_download_command") as mock_download,
):
result = runner.invoke(app, ["configure", "skills", "--mcp", "--names", "a.b.s1"])
assert result.exit_code == 1
assert "--names" in _strip_ansi(result.output)
mock_mcp.assert_not_called()
mock_download.assert_not_called()

def test_path_with_mcp_exit_1(self):
with (
patch("ucode.cli.configure_skills_mcp_command") as mock_mcp,
patch("ucode.cli.configure_location_skills_download_command") as mock_download,
):
result = runner.invoke(
app, ["configure", "skills", "--location", "a.b", "--mcp", "--path", "/tmp/skills"]
)
assert result.exit_code == 1
assert "--path" in _strip_ansi(result.output)
mock_mcp.assert_not_called()
mock_download.assert_not_called()

def test_three_part_location_exit_1(self):
with patch("ucode.cli.configure_skills_mcp_command") as mock_mcp:
result = runner.invoke(app, ["configure", "skills", "--location", "a.b.c", "--mcp"])
assert result.exit_code == 1
mock_mcp.assert_not_called()

def test_malformed_location_exit_1_names_location(self):
with patch("ucode.cli.configure_skills_mcp_command") as mock_mcp:
result = runner.invoke(app, ["configure", "skills", "--location", "justone", "--mcp"])
assert result.exit_code == 1
assert "--location" in _strip_ansi(result.output)
mock_mcp.assert_not_called()

def test_bare_command_registers_schemaless_connection(self):
with patch("ucode.cli.configure_skills_mcp_command") as mock_mcp:
result = runner.invoke(app, ["configure", "skills"])
assert result.exit_code == 0, result.output
mock_mcp.assert_called_once_with([])

def test_mcp_without_location_registers_schemaless_connection(self):
with patch("ucode.cli.configure_skills_mcp_command") as mock_mcp:
result = runner.invoke(app, ["configure", "skills", "--mcp"])
assert result.exit_code == 0, result.output
mock_mcp.assert_called_once_with([])

def test_path_without_location_exit_1(self):
with (
patch("ucode.cli.configure_skills_mcp_command") as mock_mcp,
patch("ucode.cli.configure_location_skills_download_command") as mock_download,
):
result = runner.invoke(app, ["configure", "skills", "--path", "/tmp/skills"])
assert result.exit_code == 1
assert "--path" in _strip_ansi(result.output)
mock_mcp.assert_not_called()
mock_download.assert_not_called()


class TestSkillsEntrypoint:
"""Bare `ug skills` registers the schema-less MCP connection, then prints help."""

Expand Down Expand Up @@ -2064,8 +1933,8 @@ def test_subcommand_skips_entrypoint_configuration(self):


class TestSkillsAddCommand:
"""`ucode skills add` is the additive sibling of `configure skills`: `--mcp`
unions schemas into the connection scope, the default mode downloads."""
"""`ucode skills add` is purely additive: `--mcp` unions schemas into the
connection scope, the default mode downloads."""

@pytest.fixture(autouse=True)
def _stub_install_cli(self):
Expand Down
Loading
Loading