Skip to content

Refactor: extract shared MCP setup helpers (no behavior change)#235

Draft
xsh310 wants to merge 1 commit into
databricks:mainfrom
xsh310:skills-refactor-setup-helpers
Draft

Refactor: extract shared MCP setup helpers (no behavior change)#235
xsh310 wants to merge 1 commit into
databricks:mainfrom
xsh310:skills-refactor-setup-helpers

Conversation

@xsh310

@xsh310 xsh310 commented Jul 23, 2026

Copy link
Copy Markdown

Pure, behavior-preserving refactor of src/ucode/mcp.py, prepping for the upcoming ucode configure skills rework. No behavior change: same errors, same ordering, same prints. Existing tests pass unchanged.

Extractions

  • _setup_mcp_clients(state, section) -> (workspace, profile, clients) — pulled out of configure_mcp_command: workspace validation, purge_cross_workspace_mcp_residue, installed/configured client resolution (with the two "no clients" errors), apply_pat_environment + ensure_databricks_auth, and print_section(section) + "Configuring for" note + per-missing-client warnings. configure_mcp_command now calls it with "MCP Servers".
  • _split_catalog_schema(location, flag) -> (catalog, schema) — the "exactly one dot, both parts non-empty" validation lifted out of _resolve_location_mcp_servers, which now calls it with "--location". The error message names the passed flag.
  • build_mcp_http_entry(url, *, always_load=False) — new always_load keyword sets entry["alwaysLoad"] = True when true. Default off; all existing callers unchanged; no caller uses it yet.

Tests

  • Existing MCP-command tests stay green (no-behavior-change refactor).
  • New unit tests: _split_catalog_schema accept/reject cases (message names the flag) and build_mcp_http_entry with/without always_load.

This pull request and its description were written by Isaac.

Prep for the upcoming `ucode configure skills` rework. Pure,
behavior-preserving refactor of src/ucode/mcp.py:

- Extract `_setup_mcp_clients(state, section)` from
  `configure_mcp_command` (workspace validation, cross-workspace purge,
  installed/configured client resolution, auth, and section/notes/warnings),
  returning (workspace, profile, clients).
- Extract `_split_catalog_schema(location, flag)` from
  `_resolve_location_mcp_servers`; the error message now names the flag.
- Add an `always_load` keyword to `build_mcp_http_entry`; default off, no
  caller uses it yet.

Existing tests stay green; adds unit tests for `_split_catalog_schema`
and `build_mcp_http_entry(always_load=)`.

Co-authored-by: Isaac
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.

1 participant