From f3f7453c54a3b6433eec219fd8653910835a2b02 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 21:05:10 +0000 Subject: [PATCH 1/9] configure skills: utility-tools-only mode, announce registration, restart hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three bug-bash P0 gaps in `ucode configure skills`: - Make `--location` optional. A bare `ucode configure skills` (or `--mcp` with no location) now registers the schema-less skills MCP connection — the cross-schema utility tools only — without forcing a download. `--path` without `--location` is rejected. - Announce the registration: print the server name, URL, configured agents, and a tool-category one-liner (utility tools, plus live per-schema skill tools when scoped). Download mode also prints where files landed. The summary names categories, not individual tools, so it can't drift when the backend changes its offering. - Always end with a launch/restart line ("Run `ucode ` ...; existing sessions need a restart"), and note that downloaded files already work from disk. The summary prints even on a no-op re-run; only the save is gated on a real change. Co-authored-by: Isaac --- AGENTS.md | 2 +- README.md | 30 ++++++++----- src/ucode/cli.py | 42 +++++++++---------- src/ucode/mcp.py | 79 ++++++++++++++++++++++++++++++++--- src/ucode/skills_download.py | 18 +++++--- tests/test_cli.py | 25 +++++++++-- tests/test_mcp.py | 45 ++++++++++++++++++++ tests/test_skills_download.py | 47 +++++++++++++++------ 8 files changed, 228 insertions(+), 60 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7810cb9..6a2872b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ Tests live in `tests/`. - Use Python 3.12+. - Keep changes scoped to the requested behavior. -- Follow the existing module boundaries: CLI orchestration in `cli.py`, agent-specific behavior in `agents/.py`, shared agent dispatch in `agents/__init__.py`, Databricks calls in `databricks.py`, skill download (UC fetch client + on-disk writer + download orchestration) in `skills_download.py`, MCP-connection state glue in `mcp.py`, and presentation helpers in `ui.py`. Skill download persists no disk state — it writes files to `--path` (or the home dir) and registers only the schema-less skills MCP connection. +- Follow the existing module boundaries: CLI orchestration in `cli.py`, agent-specific behavior in `agents/.py`, shared agent dispatch in `agents/__init__.py`, Databricks calls in `databricks.py`, skill download (UC fetch client + on-disk writer + download orchestration) in `skills_download.py`, MCP-connection state glue in `mcp.py`, and presentation helpers in `ui.py`. Skill download persists no disk state — it writes files to `--path` (or the home dir) and registers only the schema-less skills MCP connection. `ucode configure skills` with no `--location` (or `--mcp` with no `--location`) registers that schema-less connection without downloading anything. - Prefer existing helpers for config file writes, state persistence, UI messages, and Databricks authentication. - Add or update focused tests for behavior changes. - Do not modify generated or lock files unless the dependency graph intentionally changes. diff --git a/README.md b/README.md index d97bdeb..33cb721 100644 --- a/README.md +++ b/README.md @@ -109,25 +109,32 @@ then registers the servers); pass a comma-separated list to register several at ### Skills (optional) -Configure Unity Catalog Skills for your coding tools with `ucode configure skills`. It has two -mutually-exclusive modes, both scoped by `--location .` (comma-separated for -multiple schemas): +Configure Unity Catalog Skills for your coding tools with `ucode configure skills`: ```bash -# Download mode (default): fetch every skill in the schema to disk. +# Utility tools only: register the schema-less skills MCP connection, no download. +ucode configure skills + +# Download mode: fetch every skill in the schema to disk (and register the connection). ucode configure skills --location main.default --path /abs/project/dir # MCP mode: expose the schema's skills as MCP tools instead of downloading. ucode configure skills --location main.default,ml.prod --mcp ``` -- **Download mode** writes each skill flat as `/SKILL.md` (plus its bundled files) into both - `.claude/skills/` and `.agents/skills/`. `--path` (an existing absolute directory) is optional; - when omitted, skills are written under your home directory. Any pre-existing skill dir prompts - before it's overwritten. It then registers a schema-less skills MCP connection (utility tools - only), leaving any prior `--mcp` scope untouched. -- **MCP mode** sets the connection's location set to exactly `` (override-only) and rebuilds - its `?schema=` URL; no files are downloaded and `--path` is rejected. +- **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 directory) is optional; when omitted, skills are written under your home directory. 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. +- **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. + +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). --- @@ -145,6 +152,7 @@ ucode configure skills --location main.default,ml.prod --mcp | `ucode configure --profiles DEFAULT --use-pat` | Authenticate with the profile's personal access token — no browser login | | `ucode configure --skip-validate` | Write configs without sending a test message through each agent | | `ucode configure --agents claude --mcp system.ai.slack` | Configure an agent and register its Databricks MCP server(s) in one command | +| `ucode configure skills` | Register the schema-less skills MCP connection (utility tools only); no 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 --mcp` | Expose a schema's skills as MCP tools (override-only) instead of downloading | diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 5aa9a29..c9b2bba 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -146,11 +146,12 @@ def _parse_agents_option(agents: str) -> list[str]: return tools -def _parse_skill_locations(location: str) -> list[str]: +def _parse_skill_locations(location: str | None) -> list[str]: """Parse a comma-separated `--location` into `.` refs, - dropping duplicates while preserving order.""" + dropping duplicates while preserving order. `None`/empty yields `[]` (the + schema-less, utility-tools-only connection).""" locations: list[str] = [] - for raw in location.split(","): + for raw in (location or "").split(","): raw = raw.strip() if not raw: continue @@ -159,11 +160,6 @@ def _parse_skill_locations(location: str) -> list[str]: raise RuntimeError(f"--location entries must be `.`, got `{raw}`.") if raw not in locations: locations.append(raw) - if not locations: - raise RuntimeError( - "No schemas provided for --location. Use `.`, " - "comma-separated for multiple." - ) return locations @@ -801,8 +797,8 @@ def status() -> int: "Use `ucode configure mcp` to add Databricks MCP servers to configured coding tools." ) print_note( - "Use `ucode configure skills --location . --mcp` to connect Unity " - "Catalog Skills." + "Use `ucode configure skills` to connect Unity Catalog Skills (add " + "`--location .` to download a schema's 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.") @@ -1436,9 +1432,9 @@ def configure_mcp( @configure_app.command("skills") def configure_skills( location: Annotated[ - str, + str | None, typer.Option("--location", help="Comma-separated `.` skill scopes."), - ], + ] = None, mcp: Annotated[ bool, typer.Option("--mcp", help="Mutate the skills MCP connection instead of downloading."), @@ -1453,18 +1449,22 @@ def configure_skills( ) -> None: """Configure Databricks Skills for your coding tools. - By default, downloads every skill in each ``--location`` schema to disk - (under ``--path``, or your home dir when omitted) and registers a schema-less - MCP connection. With ``--mcp``, instead sets the skills MCP connection's scope - to exactly the listed schemas. + With no ``--location``, registers the schema-less skills MCP connection + (cross-schema utility tools only) without downloading anything. With + ``--location`` (and no ``--mcp``), also downloads every skill in each schema to + disk (under ``--path``, or your home dir when omitted). ``--mcp`` instead sets + the connection's scope to exactly the listed schemas without downloading. """ try: - if mcp: - if path is not None: - raise RuntimeError("--path is not valid with --mcp.") - configure_skills_mcp_command(_parse_skill_locations(location)) + locations = _parse_skill_locations(location) + if mcp and path is not None: + raise RuntimeError("--path is not valid with --mcp.") + 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_skills_download_command(_parse_skill_locations(location), path=path) + configure_skills_download_command(locations, path=path) except (RuntimeError, ValueError) as exc: print_err(str(exc)) raise typer.Exit(1) from None diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 2757a4b..ea14a1c 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -8,6 +8,7 @@ import string import subprocess from collections.abc import Callable +from pathlib import Path from typing import Any from urllib.parse import urlparse @@ -43,6 +44,8 @@ ) from ucode.state import load_full_state, load_state, save_state from ucode.ui import ( + print_heading, + print_kv, print_note, print_section, print_success, @@ -1445,10 +1448,61 @@ def _resolve_skills_mcp_servers( return [*kept, _build_skills_entry(workspace, locations, merged)] +def _join_human(items: list[str]) -> str: + if len(items) <= 1: + return items[0] if items else "" + return ", ".join(items[:-1]) + " and " + items[-1] + + +def _skills_tools_summary(locations: list[str]) -> str: + """One-liner describing the connection's tools. Names the tool *categories*, + not individual tools, so it never drifts when the backend changes its + offering; the per-schema skill tools are resolved live by the server.""" + if not locations: + return "UC skill utility tools" + return f"UC skill utility tools + live skills tools in schema {_join_human(locations)}" + + +def _print_skills_summary(entry: dict, *, download_roots: list[Path] | None = None) -> None: + """Report the registered skills connection: server, URL, agents, tools, and how + to start using it. ``download_roots`` (download mode) tweaks the closing line to + note that the on-disk files already work.""" + locations = entry.get("skill_locations") or [] + displays = [ + str(MCP_CLIENTS[client]["display"]) + for client in (entry.get("clients") or []) + if client in MCP_CLIENTS + ] + print_heading("Skills MCP registered") + print_kv("Server", str(entry.get("name") or SKILLS_MCP_SERVER_NAME)) + print_kv("URL", str(entry.get("url") or "")) + print_kv("Configured", ", ".join(displays) if displays else "none") + print_kv("Tools", _skills_tools_summary(locations)) + + restart = ( + "Run `ucode ` to use the skills MCP. For existing sessions, " + "restart the agent before skills become available." + ) + if download_roots is not None: + restart += ( + " Downloaded skill files already work — agents discover them from disk on next launch." + ) + print_note(restart) + + def _update_skills_mcp( - state: dict, workspace: str, profile: str | None, clients: list[str], locations: list[str] + state: dict, + workspace: str, + profile: str | None, + clients: list[str], + locations: list[str], + *, + download_roots: list[Path] | None = None, ) -> None: - """Rebuild the single skills connection for ``locations`` and persist it.""" + """Rebuild the single skills connection for ``locations``, persist it, and print + the registration summary. The summary always prints (so a no-op re-run still + reports what's registered and reminds the user to restart); only the save is + gated on an actual change.""" original = list(state.get("mcp_servers") or []) working = _resolve_skills_mcp_servers(workspace, clients, locations, original) changed = apply_mcp_server_changes(original, working, clients, workspace, profile) @@ -1456,6 +1510,8 @@ def _update_skills_mcp( state["mcp_servers"] = working save_state(state) print_success("Saved") + entry = next(s for s in working if s.get("kind") == SKILLS_MCP_KIND) + _print_skills_summary(entry, download_roots=download_roots) def configure_skills_mcp_command(locations: list[str]) -> int: @@ -1474,11 +1530,24 @@ 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], + *, + download_roots: list[Path] | None = None, ) -> None: """Register/keep the skills MCP connection without changing its schema set. Download mode calls this after writing files: it preserves any prior ``--mcp`` ``skill_locations`` and otherwise registers the bare schema-less - route (utility tools only).""" - _update_skills_mcp(state, workspace, profile, clients, _skill_mcp_locations(state)) + route (utility tools only). ``download_roots`` flows into the summary's closing + line.""" + _update_skills_mcp( + state, + workspace, + profile, + clients, + _skill_mcp_locations(state), + download_roots=download_roots, + ) diff --git a/src/ucode/skills_download.py b/src/ucode/skills_download.py index d0bc9a4..766a572 100644 --- a/src/ucode/skills_download.py +++ b/src/ucode/skills_download.py @@ -240,14 +240,15 @@ def _fetch_bundles( return results -def download_skills(workspace: str, token: str, locations: list[str], path: str | None) -> None: - """Download every skill in each ``.`` location to disk. +def download_skills(workspace: str, token: str, locations: list[str], roots: list[Path]) -> int: + """Download every skill in each ``.`` location into ``roots``. Bundles are fetched concurrently (with a progress bar) per schema, then written sequentially so overwrite prompts don't interleave. A failure on one - skill warns and skips it without aborting the batch. + skill warns and skips it without aborting the batch. Returns the total number + of skills written across all locations. """ - roots = skill_dir_roots(path) + total_written = 0 for location in locations: catalog, schema = location.split(".") leaves, reason = list_schema_skills(workspace, token, catalog, schema) @@ -267,7 +268,9 @@ def download_skills(workspace: str, token: str, locations: list[str], path: str continue if write_skill(roots, leaf, files, location=location): written += 1 + total_written += written print_success(f"Downloaded {written}/{len(leaves)} skill(s) from `{location}`.") + return total_written def configure_skills_download_command(locations: list[str], *, path: str | None) -> int: @@ -280,7 +283,10 @@ def configure_skills_download_command(locations: list[str], *, path: str | None) workspace, profile, clients = setup_mcp_clients(state, "Skills") token = get_databricks_token(workspace, profile) - download_skills(workspace, token, locations, path) + roots = skill_dir_roots(path) + written = download_skills(workspace, token, locations, roots) + if written: + print_note(f"Skill files written under {' and '.join(str(root) for root in roots)}") - register_schemaless_skills_connection(state, workspace, profile, clients) + register_schemaless_skills_connection(state, workspace, profile, clients, download_roots=roots) return 0 diff --git a/tests/test_cli.py b/tests/test_cli.py index 8e7473d..5f7f202 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -404,9 +404,28 @@ def test_malformed_location_exit_1_names_location(self): assert "--location" in _strip_ansi(result.output) mock_mcp.assert_not_called() - def test_missing_location_is_typer_usage_error(self): - result = runner.invoke(app, ["configure", "skills"]) - assert result.exit_code == 2 + 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_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 TestStatusSkillsSection: diff --git a/tests/test_mcp.py b/tests/test_mcp.py index eb39141..4fa1dea 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -4,6 +4,7 @@ import json import subprocess +from pathlib import Path from unittest.mock import MagicMock from ucode import mcp @@ -19,6 +20,11 @@ PROXY_TAIL = ["mcp-proxy", "--url", GH_URL, "--host", WS, "--profile", "p"] +def _unwrap(text: str) -> str: + """Collapse rich's line-wrapping so assertions match regardless of terminal width.""" + return " ".join(text.split()) + + def _proxy_argv() -> list[str]: from ucode.databricks import build_mcp_proxy_argv @@ -1906,6 +1912,45 @@ def test_preserves_prior_mcp_location_set(self, monkeypatch): assert _find_skills(state["mcp_servers"])[0]["skill_locations"] == ["X.x", "Y.y"] +class TestSkillsToolsSummary: + def test_bare_route_names_utility_tools_only(self): + assert mcp._skills_tools_summary([]) == "UC skill utility tools" + + def test_scoped_names_utility_plus_live_tools(self): + assert mcp._skills_tools_summary(["main.default"]) == ( + "UC skill utility tools + live skills tools in schema main.default" + ) + + def test_multiple_schemas_joined_humanly(self): + assert mcp._skills_tools_summary(["a.b", "c.d", "e.f"]) == ( + "UC skill utility tools + live skills tools in schema a.b, c.d and e.f" + ) + + +class TestPrintSkillsSummary: + def _entry(self, locations): + return mcp._resolve_skills_mcp_servers(WS, ["claude", "codex"], locations, [])[0] + + def test_reports_server_url_agents_and_tools(self, capsys): + mcp._print_skills_summary(self._entry(["main.default"])) + out = _unwrap(capsys.readouterr().out) + assert "databricks-skill-registry" in out + assert f"{WS}/ai-gateway/skills/?schema=main.default" in out + assert "Claude Code, Codex" in out + assert "live skills tools in schema main.default" in out + + def test_mcp_wording_prompts_launch_and_restart(self, capsys): + mcp._print_skills_summary(self._entry([])) + out = _unwrap(capsys.readouterr().out) + assert "ucode " in out + assert "restart the agent" in out + assert "already work" not in out + + def test_download_wording_notes_files_already_work(self, capsys): + mcp._print_skills_summary(self._entry([]), download_roots=[Path("/tmp/x/.claude/skills")]) + assert "already work" in _unwrap(capsys.readouterr().out) + + class TestRevertMcpConfigs: def test_removes_cli_registered_servers_and_restores_copilot_config(self, monkeypatch): removed: list[tuple[str, str]] = [] diff --git a/tests/test_skills_download.py b/tests/test_skills_download.py index 827bdae..9723d37 100644 --- a/tests/test_skills_download.py +++ b/tests/test_skills_download.py @@ -306,7 +306,7 @@ def test_fetches_and_writes_each_leaf(self, tmp_path, monkeypatch): sd, "fetch_skill_bundle", lambda ws, tok, c, s, leaf: (bundles[leaf], None) ) - sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) + sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) assert (tmp_path / ".claude/skills/pii-handling/SKILL.md").read_bytes() == b"pii" assert (tmp_path / ".agents/skills/triage/SKILL.md").read_bytes() == b"triage" @@ -318,7 +318,7 @@ def test_list_failure_skips_location(self, tmp_path, monkeypatch): sd, "fetch_skill_bundle", lambda *a, **k: called.append(1) or (None, None) ) - sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) + sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) assert called == [] @@ -332,7 +332,7 @@ def test_bundle_failure_skips_that_skill_only(self, tmp_path, monkeypatch): ), ) - sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) + sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) assert (tmp_path / ".claude/skills/good/SKILL.md").read_bytes() == b"ok" assert not (tmp_path / ".claude/skills/bad").exists() @@ -341,7 +341,7 @@ def test_prints_downloaded_count_summary(self, tmp_path, monkeypatch, capsys): monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: (["a", "b", "c"], None)) monkeypatch.setattr(sd, "fetch_skill_bundle", lambda *a, **k: ({"SKILL.md": b"x"}, None)) - sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) + sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) assert "Downloaded 3/3 skill(s) from `main.default`" in capsys.readouterr().out @@ -355,7 +355,7 @@ def test_summary_counts_only_written_skills(self, tmp_path, monkeypatch, capsys) ), ) - sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) + sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) assert "Downloaded 1/2 skill(s) from `main.default`" in capsys.readouterr().out @@ -371,26 +371,47 @@ def _stub(self, monkeypatch): monkeypatch.setattr( sd, "download_skills", - lambda ws, tok, locations, path: calls.update(download=(ws, tok, locations, path)), + lambda ws, tok, locations, roots: ( + calls.update(download=(ws, tok, locations, roots)) or 1 + ), ) monkeypatch.setattr( sd, "register_schemaless_skills_connection", - lambda state, ws, profile, clients: calls.update(register=(ws, profile, clients)), + lambda state, ws, profile, clients, *, download_roots: calls.update( + register=(ws, profile, clients, download_roots) + ), ) return calls - def test_downloads_then_registers_connection(self, monkeypatch): + def test_downloads_then_registers_connection(self, tmp_path, monkeypatch): calls = self._stub(monkeypatch) - assert sd.configure_skills_download_command(["a.b"], path="/tmp/skills") == 0 + assert sd.configure_skills_download_command(["a.b"], path=str(tmp_path)) == 0 - assert calls["download"] == (WS, "token", ["a.b"], "/tmp/skills") - assert calls["register"] == (WS, "profile", ["claude"]) + roots = sd.skill_dir_roots(str(tmp_path)) + assert calls["download"] == (WS, "token", ["a.b"], roots) + assert calls["register"] == (WS, "profile", ["claude"], roots) - def test_none_path_threads_through(self, monkeypatch): + def test_none_path_defaults_to_home_roots(self, monkeypatch): calls = self._stub(monkeypatch) assert sd.configure_skills_download_command(["a.b"], path=None) == 0 - assert calls["download"] == (WS, "token", ["a.b"], None) + assert calls["download"] == (WS, "token", ["a.b"], sd.skill_dir_roots(None)) + assert calls["register"] == (WS, "profile", ["claude"], sd.skill_dir_roots(None)) + + def test_prints_written_roots_when_skills_downloaded(self, tmp_path, monkeypatch, capsys): + self._stub(monkeypatch) + + sd.configure_skills_download_command(["a.b"], path=str(tmp_path)) + + assert "Skill files written under" in capsys.readouterr().out + + def test_omits_written_roots_when_nothing_downloaded(self, tmp_path, monkeypatch, capsys): + self._stub(monkeypatch) + monkeypatch.setattr(sd, "download_skills", lambda *a, **k: 0) + + sd.configure_skills_download_command(["a.b"], path=str(tmp_path)) + + assert "written under" not in capsys.readouterr().out From 6c6ddb08b92b04e4255793d037d727ef13500d77 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 21:59:10 +0000 Subject: [PATCH 2/9] Address review comments - mcp.py: drop the download-specific restart clause; rename `_join_human` to `_join_with_and`; tighten `_print_skills_summary` docstring and restore the original `_update_skills_mcp` docstring; rename the local `displays` to `clients`; remove the now-unused `download_roots` plumbing and `Path` import. - Drop "live" from the tools summary line. - skills_download.py: fold the download roots into the per-location summary ("Downloaded N/M skill(s) from `x.y` in ") instead of a separate note. - README / status hint: reword the skills copy. - Tests: assert the full summary strings (not substrings); update the download summary + dispatch tests accordingly. Co-authored-by: Isaac --- README.md | 2 +- src/ucode/cli.py | 3 +- src/ucode/mcp.py | 60 ++++++++--------------------------- src/ucode/skills_download.py | 23 ++++++-------- tests/test_mcp.py | 44 +++++++++++++------------ tests/test_skills_download.py | 58 ++++++++++++--------------------- 6 files changed, 71 insertions(+), 119 deletions(-) diff --git a/README.md b/README.md index 33cb721..d597564 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ you to run `ucode ` (existing agent sessions need a restart before the MC | `ucode configure --profiles DEFAULT --use-pat` | Authenticate with the profile's personal access token — no browser login | | `ucode configure --skip-validate` | Write configs without sending a test message through each agent | | `ucode configure --agents claude --mcp system.ai.slack` | Configure an agent and register its Databricks MCP server(s) in one command | -| `ucode configure skills` | Register the schema-less skills MCP connection (utility tools only); no download | +| `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 --mcp` | Expose a schema's skills as MCP tools (override-only) instead of downloading | diff --git a/src/ucode/cli.py b/src/ucode/cli.py index c9b2bba..d19ee30 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -797,8 +797,7 @@ def status() -> int: "Use `ucode configure mcp` to add Databricks MCP servers to configured coding tools." ) print_note( - "Use `ucode configure skills` to connect Unity Catalog Skills (add " - "`--location .` to download a schema's skills)." + "Use `ucode configure skills` to set up Unity Catalog Skills for configured coding tools." ) 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.") diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index ea14a1c..79f539f 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -8,7 +8,6 @@ import string import subprocess from collections.abc import Callable -from pathlib import Path from typing import Any from urllib.parse import urlparse @@ -1448,7 +1447,7 @@ def _resolve_skills_mcp_servers( return [*kept, _build_skills_entry(workspace, locations, merged)] -def _join_human(items: list[str]) -> str: +def _join_with_and(items: list[str]) -> str: if len(items) <= 1: return items[0] if items else "" return ", ".join(items[:-1]) + " and " + items[-1] @@ -1460,15 +1459,12 @@ def _skills_tools_summary(locations: list[str]) -> str: offering; the per-schema skill tools are resolved live by the server.""" if not locations: return "UC skill utility tools" - return f"UC skill utility tools + live skills tools in schema {_join_human(locations)}" + return f"UC skill utility tools + skills tools in schema {_join_with_and(locations)}" -def _print_skills_summary(entry: dict, *, download_roots: list[Path] | None = None) -> None: - """Report the registered skills connection: server, URL, agents, tools, and how - to start using it. ``download_roots`` (download mode) tweaks the closing line to - note that the on-disk files already work.""" - locations = entry.get("skill_locations") or [] - displays = [ +def _print_skills_summary(entry: dict) -> None: + """Report the registered skills connection and how to start using it.""" + clients = [ str(MCP_CLIENTS[client]["display"]) for client in (entry.get("clients") or []) if client in MCP_CLIENTS @@ -1476,33 +1472,18 @@ def _print_skills_summary(entry: dict, *, download_roots: list[Path] | None = No print_heading("Skills MCP registered") print_kv("Server", str(entry.get("name") or SKILLS_MCP_SERVER_NAME)) print_kv("URL", str(entry.get("url") or "")) - print_kv("Configured", ", ".join(displays) if displays else "none") - print_kv("Tools", _skills_tools_summary(locations)) - - restart = ( + print_kv("Configured", ", ".join(clients) if clients else "none") + print_kv("Tools", _skills_tools_summary(entry.get("skill_locations") or [])) + print_note( "Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) - if download_roots is not None: - restart += ( - " Downloaded skill files already work — agents discover them from disk on next launch." - ) - print_note(restart) def _update_skills_mcp( - state: dict, - workspace: str, - profile: str | None, - clients: list[str], - locations: list[str], - *, - download_roots: list[Path] | None = None, + state: dict, workspace: str, profile: str | None, clients: list[str], locations: list[str] ) -> None: - """Rebuild the single skills connection for ``locations``, persist it, and print - the registration summary. The summary always prints (so a no-op re-run still - reports what's registered and reminds the user to restart); only the save is - gated on an actual change.""" + """Rebuild the single skills connection for ``locations`` and persist it.""" original = list(state.get("mcp_servers") or []) working = _resolve_skills_mcp_servers(workspace, clients, locations, original) changed = apply_mcp_server_changes(original, working, clients, workspace, profile) @@ -1511,7 +1492,7 @@ def _update_skills_mcp( save_state(state) print_success("Saved") entry = next(s for s in working if s.get("kind") == SKILLS_MCP_KIND) - _print_skills_summary(entry, download_roots=download_roots) + _print_skills_summary(entry) def configure_skills_mcp_command(locations: list[str]) -> int: @@ -1530,24 +1511,11 @@ def _skill_mcp_locations(state: dict) -> list[str]: def register_schemaless_skills_connection( - state: dict, - workspace: str, - profile: str | None, - clients: list[str], - *, - download_roots: list[Path] | None = None, + state: dict, workspace: str, profile: str | None, clients: list[str] ) -> None: """Register/keep the skills MCP connection without changing its schema set. Download mode calls this after writing files: it preserves any prior ``--mcp`` ``skill_locations`` and otherwise registers the bare schema-less - route (utility tools only). ``download_roots`` flows into the summary's closing - line.""" - _update_skills_mcp( - state, - workspace, - profile, - clients, - _skill_mcp_locations(state), - download_roots=download_roots, - ) + route (utility tools only).""" + _update_skills_mcp(state, workspace, profile, clients, _skill_mcp_locations(state)) diff --git a/src/ucode/skills_download.py b/src/ucode/skills_download.py index 766a572..7892bb4 100644 --- a/src/ucode/skills_download.py +++ b/src/ucode/skills_download.py @@ -240,15 +240,15 @@ def _fetch_bundles( return results -def download_skills(workspace: str, token: str, locations: list[str], roots: list[Path]) -> int: - """Download every skill in each ``.`` location into ``roots``. +def download_skills(workspace: str, token: str, locations: list[str], path: str | None) -> None: + """Download every skill in each ``.`` location to disk. Bundles are fetched concurrently (with a progress bar) per schema, then written sequentially so overwrite prompts don't interleave. A failure on one - skill warns and skips it without aborting the batch. Returns the total number - of skills written across all locations. + skill warns and skips it without aborting the batch. """ - total_written = 0 + roots = skill_dir_roots(path) + roots_display = " and ".join(str(root) for root in roots) for location in locations: catalog, schema = location.split(".") leaves, reason = list_schema_skills(workspace, token, catalog, schema) @@ -268,9 +268,9 @@ def download_skills(workspace: str, token: str, locations: list[str], roots: lis continue if write_skill(roots, leaf, files, location=location): written += 1 - total_written += written - print_success(f"Downloaded {written}/{len(leaves)} skill(s) from `{location}`.") - return total_written + print_success( + f"Downloaded {written}/{len(leaves)} skill(s) from `{location}` in {roots_display}." + ) def configure_skills_download_command(locations: list[str], *, path: str | None) -> int: @@ -283,10 +283,7 @@ def configure_skills_download_command(locations: list[str], *, path: str | None) workspace, profile, clients = setup_mcp_clients(state, "Skills") token = get_databricks_token(workspace, profile) - roots = skill_dir_roots(path) - written = download_skills(workspace, token, locations, roots) - if written: - print_note(f"Skill files written under {' and '.join(str(root) for root in roots)}") + download_skills(workspace, token, locations, path) - register_schemaless_skills_connection(state, workspace, profile, clients, download_roots=roots) + register_schemaless_skills_connection(state, workspace, profile, clients) return 0 diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 4fa1dea..a58649c 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -4,7 +4,6 @@ import json import subprocess -from pathlib import Path from unittest.mock import MagicMock from ucode import mcp @@ -1916,14 +1915,14 @@ class TestSkillsToolsSummary: def test_bare_route_names_utility_tools_only(self): assert mcp._skills_tools_summary([]) == "UC skill utility tools" - def test_scoped_names_utility_plus_live_tools(self): + def test_scoped_names_utility_plus_skills_tools(self): assert mcp._skills_tools_summary(["main.default"]) == ( - "UC skill utility tools + live skills tools in schema main.default" + "UC skill utility tools + skills tools in schema main.default" ) - def test_multiple_schemas_joined_humanly(self): + def test_multiple_schemas_joined_with_and(self): assert mcp._skills_tools_summary(["a.b", "c.d", "e.f"]) == ( - "UC skill utility tools + live skills tools in schema a.b, c.d and e.f" + "UC skill utility tools + skills tools in schema a.b, c.d and e.f" ) @@ -1931,24 +1930,29 @@ class TestPrintSkillsSummary: def _entry(self, locations): return mcp._resolve_skills_mcp_servers(WS, ["claude", "codex"], locations, [])[0] - def test_reports_server_url_agents_and_tools(self, capsys): + def test_reports_scoped_connection(self, capsys): mcp._print_skills_summary(self._entry(["main.default"])) - out = _unwrap(capsys.readouterr().out) - assert "databricks-skill-registry" in out - assert f"{WS}/ai-gateway/skills/?schema=main.default" in out - assert "Claude Code, Codex" in out - assert "live skills tools in schema main.default" in out + assert _unwrap(capsys.readouterr().out) == ( + "Skills MCP registered " + "Server: databricks-skill-registry " + f"URL: {WS}/ai-gateway/skills/?schema=main.default " + "Configured: Claude Code, Codex " + "Tools: UC skill utility tools + skills tools in schema main.default " + "• Run `ucode ` to use the skills MCP. For existing sessions, " + "restart the agent before skills become available." + ) - def test_mcp_wording_prompts_launch_and_restart(self, capsys): + def test_reports_schemaless_connection(self, capsys): mcp._print_skills_summary(self._entry([])) - out = _unwrap(capsys.readouterr().out) - assert "ucode " in out - assert "restart the agent" in out - assert "already work" not in out - - def test_download_wording_notes_files_already_work(self, capsys): - mcp._print_skills_summary(self._entry([]), download_roots=[Path("/tmp/x/.claude/skills")]) - assert "already work" in _unwrap(capsys.readouterr().out) + assert _unwrap(capsys.readouterr().out) == ( + "Skills MCP registered " + "Server: databricks-skill-registry " + f"URL: {WS}/ai-gateway/skills/ " + "Configured: Claude Code, Codex " + "Tools: UC skill utility tools " + "• Run `ucode ` to use the skills MCP. For existing sessions, " + "restart the agent before skills become available." + ) class TestRevertMcpConfigs: diff --git a/tests/test_skills_download.py b/tests/test_skills_download.py index 9723d37..33ccf62 100644 --- a/tests/test_skills_download.py +++ b/tests/test_skills_download.py @@ -306,7 +306,7 @@ def test_fetches_and_writes_each_leaf(self, tmp_path, monkeypatch): sd, "fetch_skill_bundle", lambda ws, tok, c, s, leaf: (bundles[leaf], None) ) - sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) + sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) assert (tmp_path / ".claude/skills/pii-handling/SKILL.md").read_bytes() == b"pii" assert (tmp_path / ".agents/skills/triage/SKILL.md").read_bytes() == b"triage" @@ -318,7 +318,7 @@ def test_list_failure_skips_location(self, tmp_path, monkeypatch): sd, "fetch_skill_bundle", lambda *a, **k: called.append(1) or (None, None) ) - sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) + sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) assert called == [] @@ -332,18 +332,22 @@ def test_bundle_failure_skips_that_skill_only(self, tmp_path, monkeypatch): ), ) - sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) + sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) assert (tmp_path / ".claude/skills/good/SKILL.md").read_bytes() == b"ok" assert not (tmp_path / ".claude/skills/bad").exists() - def test_prints_downloaded_count_summary(self, tmp_path, monkeypatch, capsys): + def test_prints_downloaded_count_and_roots_summary(self, tmp_path, monkeypatch, capsys): monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: (["a", "b", "c"], None)) monkeypatch.setattr(sd, "fetch_skill_bundle", lambda *a, **k: ({"SKILL.md": b"x"}, None)) - sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) + sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) - assert "Downloaded 3/3 skill(s) from `main.default`" in capsys.readouterr().out + # Rich wraps long paths across lines; strip all whitespace from both sides to compare. + roots = sd.skill_dir_roots(str(tmp_path)) + expected = f"Downloaded 3/3 skill(s) from `main.default` in {roots[0]} and {roots[1]}." + printed = "".join(capsys.readouterr().out.split()) + assert "".join(expected.split()) in printed def test_summary_counts_only_written_skills(self, tmp_path, monkeypatch, capsys): monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: (["good", "bad"], None)) @@ -355,9 +359,9 @@ def test_summary_counts_only_written_skills(self, tmp_path, monkeypatch, capsys) ), ) - sd.download_skills(WS, "token", ["main.default"], sd.skill_dir_roots(str(tmp_path))) + sd.download_skills(WS, "token", ["main.default"], str(tmp_path)) - assert "Downloaded 1/2 skill(s) from `main.default`" in capsys.readouterr().out + assert "Downloaded 1/2 skill(s) from `main.default` in" in capsys.readouterr().out class TestConfigureSkillsDownloadCommand: @@ -371,47 +375,27 @@ def _stub(self, monkeypatch): monkeypatch.setattr( sd, "download_skills", - lambda ws, tok, locations, roots: ( - calls.update(download=(ws, tok, locations, roots)) or 1 - ), + lambda ws, tok, locations, path: calls.update(download=(ws, tok, locations, path)), ) monkeypatch.setattr( sd, "register_schemaless_skills_connection", - lambda state, ws, profile, clients, *, download_roots: calls.update( - register=(ws, profile, clients, download_roots) - ), + lambda state, ws, profile, clients: calls.update(register=(ws, profile, clients)), ) return calls - def test_downloads_then_registers_connection(self, tmp_path, monkeypatch): + def test_downloads_then_registers_connection(self, monkeypatch): calls = self._stub(monkeypatch) - assert sd.configure_skills_download_command(["a.b"], path=str(tmp_path)) == 0 + assert sd.configure_skills_download_command(["a.b"], path="/tmp/skills") == 0 - roots = sd.skill_dir_roots(str(tmp_path)) - assert calls["download"] == (WS, "token", ["a.b"], roots) - assert calls["register"] == (WS, "profile", ["claude"], roots) + assert calls["download"] == (WS, "token", ["a.b"], "/tmp/skills") + assert calls["register"] == (WS, "profile", ["claude"]) - def test_none_path_defaults_to_home_roots(self, monkeypatch): + def test_none_path_threads_through(self, monkeypatch): calls = self._stub(monkeypatch) assert sd.configure_skills_download_command(["a.b"], path=None) == 0 - assert calls["download"] == (WS, "token", ["a.b"], sd.skill_dir_roots(None)) - assert calls["register"] == (WS, "profile", ["claude"], sd.skill_dir_roots(None)) - - def test_prints_written_roots_when_skills_downloaded(self, tmp_path, monkeypatch, capsys): - self._stub(monkeypatch) - - sd.configure_skills_download_command(["a.b"], path=str(tmp_path)) - - assert "Skill files written under" in capsys.readouterr().out - - def test_omits_written_roots_when_nothing_downloaded(self, tmp_path, monkeypatch, capsys): - self._stub(monkeypatch) - monkeypatch.setattr(sd, "download_skills", lambda *a, **k: 0) - - sd.configure_skills_download_command(["a.b"], path=str(tmp_path)) - - assert "written under" not in capsys.readouterr().out + assert calls["download"] == (WS, "token", ["a.b"], None) + assert calls["register"] == (WS, "profile", ["claude"]) From 7391f52eb9d10f0a7d81734b975bbfc655ec45a7 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:08:12 +0000 Subject: [PATCH 3/9] Address review comments: rename tools helper, add status markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename `_skills_tools_summary` to `_skills_tools_description` and drop its docstring (the function is self-explanatory). - Prefix the summary heading with ✔ and the restart note with ⚠. Co-authored-by: Isaac --- src/ucode/mcp.py | 11 ++++------- tests/test_mcp.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 79f539f..3ce7efd 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -1453,10 +1453,7 @@ def _join_with_and(items: list[str]) -> str: return ", ".join(items[:-1]) + " and " + items[-1] -def _skills_tools_summary(locations: list[str]) -> str: - """One-liner describing the connection's tools. Names the tool *categories*, - not individual tools, so it never drifts when the backend changes its - offering; the per-schema skill tools are resolved live by the server.""" +def _skills_tools_description(locations: list[str]) -> str: if not locations: return "UC skill utility tools" return f"UC skill utility tools + skills tools in schema {_join_with_and(locations)}" @@ -1469,13 +1466,13 @@ def _print_skills_summary(entry: dict) -> None: for client in (entry.get("clients") or []) if client in MCP_CLIENTS ] - print_heading("Skills MCP registered") + print_heading("✔ Skills MCP registered") print_kv("Server", str(entry.get("name") or SKILLS_MCP_SERVER_NAME)) print_kv("URL", str(entry.get("url") or "")) print_kv("Configured", ", ".join(clients) if clients else "none") - print_kv("Tools", _skills_tools_summary(entry.get("skill_locations") or [])) + print_kv("Tools", _skills_tools_description(entry.get("skill_locations") or [])) print_note( - "Run `ucode ` to use the skills MCP. For existing sessions, " + "⚠ Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) diff --git a/tests/test_mcp.py b/tests/test_mcp.py index a58649c..6054032 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -1911,17 +1911,17 @@ def test_preserves_prior_mcp_location_set(self, monkeypatch): assert _find_skills(state["mcp_servers"])[0]["skill_locations"] == ["X.x", "Y.y"] -class TestSkillsToolsSummary: +class TestSkillsToolsDescription: def test_bare_route_names_utility_tools_only(self): - assert mcp._skills_tools_summary([]) == "UC skill utility tools" + assert mcp._skills_tools_description([]) == "UC skill utility tools" def test_scoped_names_utility_plus_skills_tools(self): - assert mcp._skills_tools_summary(["main.default"]) == ( + assert mcp._skills_tools_description(["main.default"]) == ( "UC skill utility tools + skills tools in schema main.default" ) def test_multiple_schemas_joined_with_and(self): - assert mcp._skills_tools_summary(["a.b", "c.d", "e.f"]) == ( + assert mcp._skills_tools_description(["a.b", "c.d", "e.f"]) == ( "UC skill utility tools + skills tools in schema a.b, c.d and e.f" ) @@ -1933,24 +1933,24 @@ def _entry(self, locations): def test_reports_scoped_connection(self, capsys): mcp._print_skills_summary(self._entry(["main.default"])) assert _unwrap(capsys.readouterr().out) == ( - "Skills MCP registered " + "✔ Skills MCP registered " "Server: databricks-skill-registry " f"URL: {WS}/ai-gateway/skills/?schema=main.default " "Configured: Claude Code, Codex " "Tools: UC skill utility tools + skills tools in schema main.default " - "• Run `ucode ` to use the skills MCP. For existing sessions, " + "• ⚠ Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) def test_reports_schemaless_connection(self, capsys): mcp._print_skills_summary(self._entry([])) assert _unwrap(capsys.readouterr().out) == ( - "Skills MCP registered " + "✔ Skills MCP registered " "Server: databricks-skill-registry " f"URL: {WS}/ai-gateway/skills/ " "Configured: Claude Code, Codex " "Tools: UC skill utility tools " - "• Run `ucode ` to use the skills MCP. For existing sessions, " + "• ⚠ Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) From 7434ac6c750774a0d88f126b3de40406c7112cda Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:11:56 +0000 Subject: [PATCH 4/9] =?UTF-8?q?Drop=20the=20=E2=9A=A0=20marker=20from=20th?= =?UTF-8?q?e=20restart=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the plain `print_note` bullet for the restart line; the ✔ heading stays. Co-authored-by: Isaac --- src/ucode/mcp.py | 2 +- tests/test_mcp.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 3ce7efd..96ec2e8 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -1472,7 +1472,7 @@ def _print_skills_summary(entry: dict) -> None: print_kv("Configured", ", ".join(clients) if clients else "none") print_kv("Tools", _skills_tools_description(entry.get("skill_locations") or [])) print_note( - "⚠ Run `ucode ` to use the skills MCP. For existing sessions, " + "Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 6054032..8169881 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -1938,7 +1938,7 @@ def test_reports_scoped_connection(self, capsys): f"URL: {WS}/ai-gateway/skills/?schema=main.default " "Configured: Claude Code, Codex " "Tools: UC skill utility tools + skills tools in schema main.default " - "• ⚠ Run `ucode ` to use the skills MCP. For existing sessions, " + "• Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) @@ -1950,7 +1950,7 @@ def test_reports_schemaless_connection(self, capsys): f"URL: {WS}/ai-gateway/skills/ " "Configured: Claude Code, Codex " "Tools: UC skill utility tools " - "• ⚠ Run `ucode ` to use the skills MCP. For existing sessions, " + "• Run `ucode ` to use the skills MCP. For existing sessions, " "restart the agent before skills become available." ) From 6179337d903816abb5b67607ce20f0ba89e03515 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:23:08 +0000 Subject: [PATCH 5/9] Drop the redundant "Saved" line from the skills summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `✔ Skills MCP registered` summary heading now serves as the confirmation, so the separate `✔ Saved` line was printing two checkmarks for one action. Co-authored-by: Isaac --- src/ucode/mcp.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 96ec2e8..8fe6afa 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -1487,7 +1487,6 @@ def _update_skills_mcp( if changed or original != working: state["mcp_servers"] = working save_state(state) - print_success("Saved") entry = next(s for s in working if s.get("kind") == SKILLS_MCP_KIND) _print_skills_summary(entry) From 3b96baf45db688a612c0bb1dfacfb60b2412c802 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:25:55 +0000 Subject: [PATCH 6/9] Address review comments: reword skills docstring and restart note - Restructure the `configure_skills` docstring around whether `--location` is given, then whether `--mcp` is present. - Reword the restart note to "restart the agent for the skills to take effect". Co-authored-by: Isaac --- src/ucode/cli.py | 12 +++++++----- src/ucode/mcp.py | 2 +- tests/test_mcp.py | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index d19ee30..20dd8fb 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1448,11 +1448,13 @@ def configure_skills( ) -> None: """Configure Databricks Skills for your coding tools. - With no ``--location``, registers the schema-less skills MCP connection - (cross-schema utility tools only) without downloading anything. With - ``--location`` (and no ``--mcp``), also downloads every skill in each schema to - disk (under ``--path``, or your home dir when omitted). ``--mcp`` instead sets - the connection's scope to exactly the listed schemas without downloading. + 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 schema-less connection. """ try: locations = _parse_skill_locations(location) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 8fe6afa..367b726 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -1473,7 +1473,7 @@ def _print_skills_summary(entry: dict) -> None: print_kv("Tools", _skills_tools_description(entry.get("skill_locations") or [])) print_note( "Run `ucode ` to use the skills MCP. For existing sessions, " - "restart the agent before skills become available." + "restart the agent for the skills to take effect." ) diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 8169881..981dccc 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -1939,7 +1939,7 @@ def test_reports_scoped_connection(self, capsys): "Configured: Claude Code, Codex " "Tools: UC skill utility tools + skills tools in schema main.default " "• Run `ucode ` to use the skills MCP. For existing sessions, " - "restart the agent before skills become available." + "restart the agent for the skills to take effect." ) def test_reports_schemaless_connection(self, capsys): @@ -1951,7 +1951,7 @@ def test_reports_schemaless_connection(self, capsys): "Configured: Claude Code, Codex " "Tools: UC skill utility tools " "• Run `ucode ` to use the skills MCP. For existing sessions, " - "restart the agent before skills become available." + "restart the agent for the skills to take effect." ) From 7f8062133884def9ae3d0c1b59ebb15953bf8b87 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:27:02 +0000 Subject: [PATCH 7/9] Clarify download-path docstring wording The download path registers the MCP connection with utility tools only. Co-authored-by: Isaac --- src/ucode/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 20dd8fb..c958d70 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1454,7 +1454,7 @@ def configure_skills( 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 schema-less connection. + registers the MCP connection with utility tools only. """ try: locations = _parse_skill_locations(location) From 11a938fc61907aa14be1f0d4a94172b0f121f6fb Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:31:32 +0000 Subject: [PATCH 8/9] Match the summary checkmark color and space the download line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Color the summary heading's ✔ green so it matches the green ✔ from `print_success` on the download line (the heading's bold-wrap was rendering it white). - Print a blank line before each per-location "Downloaded ..." line. Co-authored-by: Isaac --- src/ucode/mcp.py | 2 +- src/ucode/skills_download.py | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 367b726..9915466 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -1466,7 +1466,7 @@ def _print_skills_summary(entry: dict) -> None: for client in (entry.get("clients") or []) if client in MCP_CLIENTS ] - print_heading("✔ Skills MCP registered") + print_heading("[green]✔[/green] Skills MCP registered") print_kv("Server", str(entry.get("name") or SKILLS_MCP_SERVER_NAME)) print_kv("URL", str(entry.get("url") or "")) print_kv("Configured", ", ".join(clients) if clients else "none") diff --git a/src/ucode/skills_download.py b/src/ucode/skills_download.py index 7892bb4..8822640 100644 --- a/src/ucode/skills_download.py +++ b/src/ucode/skills_download.py @@ -15,7 +15,14 @@ ) from ucode.mcp import register_schemaless_skills_connection, setup_mcp_clients from ucode.state import load_state -from ucode.ui import print_note, print_success, print_warning, progress_bar, prompt_yes_no +from ucode.ui import ( + console, + print_note, + print_success, + print_warning, + progress_bar, + prompt_yes_no, +) # `.claude/skills` (Claude) + `.agents/skills` (the alias other agents read). SKILL_BASE_DIR_NAMES = (".claude/skills", ".agents/skills") @@ -268,6 +275,7 @@ def download_skills(workspace: str, token: str, locations: list[str], path: str continue if write_skill(roots, leaf, files, location=location): written += 1 + console.print() print_success( f"Downloaded {written}/{len(leaves)} skill(s) from `{location}` in {roots_display}." ) From aa8237bb955cc46a9e9d72a4dd4ccc8b37b25dd3 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Mon, 27 Jul 2026 22:34:17 +0000 Subject: [PATCH 9/9] Use print_success for the skills summary heading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the manually-greened print_heading with print_success so the summary heading's ✔ matches the download line's checkmark (both green, no bold-wrap or inline color markup). Keep the leading blank line for spacing. Co-authored-by: Isaac --- src/ucode/mcp.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 9915466..c8aa0c5 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -43,7 +43,7 @@ ) from ucode.state import load_full_state, load_state, save_state from ucode.ui import ( - print_heading, + console, print_kv, print_note, print_section, @@ -1466,7 +1466,8 @@ def _print_skills_summary(entry: dict) -> None: for client in (entry.get("clients") or []) if client in MCP_CLIENTS ] - print_heading("[green]✔[/green] Skills MCP registered") + console.print() + print_success("Skills MCP registered") print_kv("Server", str(entry.get("name") or SKILLS_MCP_SERVER_NAME)) print_kv("URL", str(entry.get("url") or "")) print_kv("Configured", ", ".join(clients) if clients else "none")