Skip to content

ucode configure skills: MCP-connection mode (add/remove/replace) + status#236

Merged
AarushiShah-db merged 8 commits into
databricks:mainfrom
xsh310:skills-mcp-mode
Jul 24, 2026
Merged

ucode configure skills: MCP-connection mode (add/remove/replace) + status#236
AarushiShah-db merged 8 commits into
databricks:mainfrom
xsh310:skills-mcp-mode

Conversation

@xsh310

@xsh310 xsh310 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds ucode configure skills for managing a single Unity Catalog Skills MCP connection (databricks-skill-registry) whose scope is a set of catalog.schema locations, rendered as a multi-schema ?schema= URL (aligned to the backend route in databricks-eng/universe#2256555). Also adds a top-level Skills section to ucode status.

Self-contained PR off main (a small _setup_mcp_clients / build_mcp_http_entry refactor that was originally split into a separate PR has been folded in here).

What's in this PR

  • build_skills_mcp_url(workspace, locations) — multi-schema ?schema=a&schema=b; empty list → bare /ai-gateway/skills/ (utility tools only).
  • Single skills connection (kind:"skills", fixed name databricks-skill-registry) with skill_locations as the source of truth; the URL is derived, never parsed back. Every run rebuilds exactly one skills entry (single-connection invariant); non-skills entries are preserved.
  • ucode configure skills --location a.b,c.d --mcp — sets the connection's location set to exactly the listed schemas, replacing any prior set. Override-only for now: to add or drop a schema, re-pass the full list you want. (--remove/--replace mutation flags are intentionally out of scope to keep the surface minimal.)
  • MCP mode is required for now: a bare ucode configure skills --location a.b (no --mcp) raises a clear "Download mode is not available yet; pass --mcp for now." error rather than silently accepting a no-op. Download mode is a planned follow-up.
  • alwaysLoad is set on the skills entry for the Claude client only (rides the existing per-client apply); other clients get the URL only.
  • Behavior-preserving prep folded in: _setup_mcp_clients extracted from configure_mcp_command (shared by the mcp and skills commands), and an always_load keyword added to build_mcp_http_entry (default off).
  • ucode status gains a Skills section (Skill MCP Locations + Configured agents); the skills entry is excluded from each agent's per-client MCP-servers line to avoid double-listing.

How do you know it works?

Unit tests

Unit tests across tests/test_databricks.py, tests/test_mcp.py, tests/test_cli.py:

  • build_skills_mcp_url for zero / one / many locations (order preserved).
  • _resolve_skills_mcp_servers single-entry rebuild, URL derived from skill_locations, mcp-service entries preserved, alwaysLoad set.
  • configure_skills_mcp_command flows: set-on-empty (registers connection), --location replaces a prior set, multiple locations set in order, mcp-service entries preserved across a set.
  • CLI dispatch: --mcp sets the location set, comma --location parsing, --mcp required (download-mode error without it), 3-part .skill rejected under --mcp, malformed → error, missing --location → usage error; status Skills-section renderings.

Verified locally with uv run ruff check . (clean) and the full pytest suite: 915 passed, 39 skipped, 1 failed — the single failure is a pre-existing, environment-only test_e2e_user_agent.py case (the local claude binary is Claude Code, not the standalone Anthropic CLI) that also fails on main.

Manual E2E verification (live workspace)

Ran the CLI end to end against a real workspace, eng-ml-inference.staging (profile eng-ml-inference-staging), with Claude Code and Codex configured as agents. For each step I checked the ~/.ucode/state.json skills entry, ucode status, and the agent registrations via claude mcp list / codex mcp list. (The runs below were against the earlier add/remove/replace surface; the override semantics are what this PR now ships — the set-and-render, no-clobber, endpoint-reachability, and error-path behaviors they exercised are unchanged, and the override behavior is covered by the updated unit tests. A fresh manual pass on the override-only surface is planned before undrafting.)

Step Command Result
Set one configure skills --location xsh.uc_skill_test --mcp skill_locations: ["xsh.uc_skill_test"], url .../ai-gateway/skills/?schema=xsh.uc_skill_test, entry named databricks-skill-registry, clients [claude, codex]; both agents registered. A pre-existing old-format kind:"skills" entry was swept into the canonical shape.
Set two (multi-schema) --location xsh.uc_skill_test,xsh.xsh_oncall_skills --mcp Set is ["xsh.uc_skill_test", "xsh.xsh_oncall_skills"]; URL carries both &schema= params in order.
Override with one --location xsh.uc_skill_test --mcp Set becomes exactly ["xsh.uc_skill_test"]; both agent registrations updated.
Coexistence / no-clobber configure mcp --location system.ai with a skills entry present The 10 added system.ai mcp-service entries and the skills entry coexist; the skills entry survives in state, in ucode status, and in the Claude registration.
Error paths no --mcp; 3-part a.b.c --mcp; malformed foo --mcp All exit 1 with the expected messages ("Download mode is not available yet; pass --mcp for now.", "--location entries must be <catalog>.<schema>..."); a valid set exits 0.

ucode status Skills section rendered as expected throughout, e.g.:

Skills
  Skill MCP Locations: xsh.uc_skill_test, xsh.xsh_oncall_skills
  Configured: Claude Code, Codex

Endpoint reachability. Posting a tools/list JSON-RPC body to the URL ucode renders returned real tools with HTTP 200, directly against the plain workspace endpoint (the multi-schema route from databricks-eng/universe#2256555 is already deployed to this staging workspace, so no LiteSwap traffic header was needed):

  • Single scope ?schema=xsh.uc_skill_test → 3 skill tools plus the 6 utility tools (load_skill, get_skill_files, list_skills, create_skill, update_skill, delete_skill).
  • Two-scope set → 3 + 5 = 8 skill tools plus the 6 utility tools.
  • Bare /ai-gateway/skills/ → 0 skill tools, the 6 utility tools only.

Note: the deployed handler currently emits full-FQN skill tool names (skill_<catalog>.<schema>.<leaf>) for both single and multi-scope requests. This is a gateway-side detail; ucode only renders the connection URL, so it does not affect this PR.

Screenshot 2026-07-23 at 12 49 52 PM Screenshot 2026-07-23 at 12 46 02 PM

This pull request and its description were written by Isaac.

Introduces `ucode configure skills` for managing a single Unity Catalog
Skills MCP connection (`databricks-skill-registry`) whose scope is a set
of `catalog.schema` locations, rendered as a multi-schema `?schema=` URL
(aligns with the backend route in universe #2256555).

- `build_skills_mcp_url(workspace, locations)`: repeated `?schema=`; empty
  list -> bare `/ai-gateway/skills/` (utility tools only).
- Single `kind:"skills"` entry with `skill_locations` as the source of
  truth; the URL is derived and never parsed back. Every run rebuilds
  exactly one skills entry, preserving non-skills entries.
- `--location a.b,c.d --mcp [--remove|--replace]` adds/removes/replaces the
  location set; `--remove`/`--replace` are mutually exclusive and require
  `--mcp`; a soft warning fires above 10 schemas. The bare form (no
  `--mcp`) registers the connection for the given schemas; download-only
  affordances (`--path`, `--yes`, a 3-part `.skill` location) without
  `--mcp` raise a clear "not available yet" error.
- `alwaysLoad` is set on the skills entry for the Claude client only.
- `ucode status` gains a top-level Skills section (Connection / Mode /
  Locations / Configured); the skills entry is excluded from each agent's
  per-client MCP-servers line.

Also extracts `_setup_mcp_clients` from `configure_mcp_command` (shared by
the mcp and skills commands) and adds an `always_load` keyword to
`build_mcp_http_entry` (default off) as behavior-preserving prep.

Co-authored-by: Isaac
xsh310 added 2 commits July 23, 2026 16:55
`configure mcp --location` (and the interactive picker) rebuilt the MCP
server list from only the mcp-services they manage, so `apply_mcp_server_changes`
treated the `kind:"skills"` entry as a removal and unregistered it from every
agent. Both paths now carry an existing skills connection through untouched
(the picker also hides it, since it is managed by `configure skills`).

Co-authored-by: Isaac
Drop the Connection line (fixed name, no signal) and the Mode line
(inferred; deferred until download mode lands), and rename the locations
line to `Skill MCP Locations` to distinguish it from download locations a
later PR may track separately.

Co-authored-by: Isaac
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/cli.py Outdated
Comment thread src/ucode/databricks.py Outdated
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
Defer all download scaffolding to the later download-mode PR and apply
review cleanups:

- CLI: drop --path/--yes and 3-part `.skill` parsing; --location takes
  only `<catalog>.<schema>` refs. Non---mcp invocations return a clear
  "not available yet" error until download mode lands.
- Remove the bare `configure_skills_command` entry point and the
  `SkillLocation` NamedTuple.
- Drop the client-side schema soft-cap; let the backend enforce limits.
- Rename `_apply_skills_connection` -> `_update_skills_mcp`; group the
  skills helpers just above `configure_skills_mcp_command`.
- `resolve_skill_location_set` dedupes via `dict.fromkeys`.
- status: rename locals to `skill_mcp_entry` / `configured_agents`.
- Tighten `build_skills_mcp_url` docstring and a couple of comments.

Co-authored-by: Isaac
Comment thread src/ucode/mcp.py Outdated
Comment thread src/ucode/mcp.py Outdated
- `resolve_skill_location_set` no longer de-dupes the `replace` set; the
  CLI parser (`_parse_skill_locations`) already drops duplicates, so it
  just returns `requested`.
- Move `_merge_clients`, `_build_skills_entry`, and
  `_resolve_skills_mcp_servers` down to sit with the rest of the skills
  helpers just above `configure_skills_mcp_command`.

Co-authored-by: Isaac
@xsh310
xsh310 marked this pull request as ready for review July 23, 2026 19:51
xsh310 added 2 commits July 23, 2026 21:54
Per review, keep the MCP-mode surface minimal: --location now sets the
skills connection's schema set to exactly the listed locations, replacing
any prior set, instead of add/remove/replace mutations. Removes the
--remove/--replace flags and mode dispatch, and the now-dead
resolve_skill_location_set / _current_skill_locations helpers.

Co-authored-by: Isaac
Surface `ucode configure skills --location <catalog>.<schema> --mcp`
alongside the existing configure-mcp/tracing hints so users discover the
Skills connection command from `ucode status`.

Co-authored-by: Isaac
@AarushiShah-db
AarushiShah-db merged commit 0fed5fd into databricks:main Jul 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants