From 7d015ef496592412f3c7ad591fab5aa95bba2404 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 6 Aug 2026 05:02:49 +0000 Subject: [PATCH 1/3] Make the Databricks CLI version update a no-op ucode used to read `databricks --version` and, when it was below MIN_DATABRICKS_CLI_VERSION, shell out to brew/curl/wget (with sudo on Linux) to replace the user's install. Silently swapping a CLI the user manages themselves -- including a locally built one -- is more disruptive than letting a feature fail with the CLI's own error message. `ensure_databricks_cli_version()` is now a no-op, and the bootstrap path (renamed `install_databricks_cli` -> `ensure_databricks_cli`) only checks PATH, raising with install instructions instead of installing. Drops the installer helper, the version parser, and the min-version constant; the README now lists the CLI as a prerequisite. Co-authored-by: Isaac --- README.md | 1 + src/ucode/agents/__init__.py | 4 +- src/ucode/cli.py | 8 ++-- src/ucode/databricks.py | 90 +++++++----------------------------- tests/test_cli.py | 64 ++++++++++++------------- tests/test_databricks.py | 87 ++++++++++++---------------------- 6 files changed, 85 insertions(+), 169 deletions(-) diff --git a/README.md b/README.md index 0352757..4e0c849 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ ## Requirements - Python 3.12+ — install with `uv` ([uv.astral.sh](https://docs.astral.sh/uv/getting-started/installation/)) +- The Databricks CLI on your `PATH` ([install guide](https://docs.databricks.com/aws/en/dev-tools/cli/install)) — `ucode` never installs or upgrades it for you. Keep it current (`brew upgrade databricks/tap/databricks`); features like `ucode configure` rely on recent subcommands such as `databricks aitools`. - `npm` if tool CLIs need to be installed automatically ## Installation diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index 3afd4fe..598c16c 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -19,9 +19,9 @@ from ucode.config_io import ToolSpec from ucode.databricks import ( BEDROCK_PROVIDER_TYPES, + ensure_databricks_cli, get_databricks_token, install_ai_tools, - install_databricks_cli, map_bedrock_claude_models, resolve_provider_service, ) @@ -254,7 +254,7 @@ def ensure_bootstrap_dependencies( update_existing: bool = False, prompt_optional_updates: bool = True, ) -> None: - install_databricks_cli() + ensure_databricks_cli() install_tool_binary( tool, strict=True, diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 58a4e4a..3f7a0d2 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -43,11 +43,11 @@ discover_model_services, ensure_ai_gateway_v2, ensure_databricks_auth, + ensure_databricks_cli, ensure_pat_bearer, find_profile_name_for_host, get_databricks_profiles, get_databricks_token, - install_databricks_cli, is_model_provider_feature_unavailable, list_profile_entries, list_tool_provider_services, @@ -1648,7 +1648,7 @@ def configure( set_verbosity(verbose) prompt_optional_updates = not skip_upgrade try: - install_databricks_cli() + ensure_databricks_cli() if agent is not None and agents is not None: raise RuntimeError("Use either --agent or --agents, not both.") if workspaces is not None and profiles is not None: @@ -1927,7 +1927,7 @@ def configure_tracing( ) -> None: """Send coding-session traces to an MLflow experiment in your workspace.""" try: - install_databricks_cli() + ensure_databricks_cli() configure_tracing_command(disable=disable) except RuntimeError as exc: print_err(str(exc)) @@ -1961,7 +1961,7 @@ def revert_cmd() -> None: def usage_cmd() -> None: """Show Databricks AI Gateway usage summary (last 7 days).""" try: - install_databricks_cli() + ensure_databricks_cli() usage_report() except RuntimeError as exc: print_err(str(exc)) diff --git a/src/ucode/databricks.py b/src/ucode/databricks.py index 0a004a1..a7d53dc 100644 --- a/src/ucode/databricks.py +++ b/src/ucode/databricks.py @@ -43,15 +43,8 @@ spinner, ) -UNIX_DATABRICKS_INSTALL_URL = ( - "https://raw.githubusercontent.com/databricks/setup-cli/main/install.sh" -) -WINDOWS_DATABRICKS_INSTALL_URL = ( - "https://raw.githubusercontent.com/databricks/setup-cli/main/install.ps1" -) AI_GATEWAY_V2_DOCS_URL = "https://docs.databricks.com/aws/en/ai-gateway/overview-beta" -# v1.0.0 is the release that ships `databricks aitools`. -MIN_DATABRICKS_CLI_VERSION = (1, 0, 0) +DATABRICKS_CLI_INSTALL_DOCS_URL = "https://docs.databricks.com/aws/en/dev-tools/cli/install" TOKEN_REFRESH_INTERVAL_SECONDS = 1800 @@ -517,75 +510,26 @@ def workspace_hostname(workspace: str) -> str: return parsed.hostname -def _parse_databricks_cli_version(output: str) -> tuple[int, int, int] | None: - # Example output: "Databricks CLI v0.299.2" - match = re.search(r"v?(\d+)\.(\d+)\.(\d+)", output) - if not match: - return None - return (int(match.group(1)), int(match.group(2)), int(match.group(3))) - - -def _run_databricks_cli_installer(brew_subcommand: str = "install") -> None: - system = platform.system() - try: - if system == "Windows": - run( - ["powershell", "-Command", f"irm {WINDOWS_DATABRICKS_INSTALL_URL} | iex"], - timeout=240, - ) - elif system == "Darwin" and shutil.which("brew"): - run(["brew", brew_subcommand, "databricks/tap/databricks"], timeout=240) - elif shutil.which("curl"): - run(["sh", "-c", f"curl -fsSL {UNIX_DATABRICKS_INSTALL_URL} | sudo sh"], timeout=240) - elif shutil.which("wget"): - run(["sh", "-c", f"wget -qO- {UNIX_DATABRICKS_INSTALL_URL} | sudo sh"], timeout=240) - else: - raise RuntimeError("Neither curl nor wget is available.") - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, RuntimeError) as exc: - raise RuntimeError("Failed to install/upgrade Databricks CLI automatically.") from exc - - def ensure_databricks_cli_version() -> None: - try: - result = run( - ["databricks", "--version"], - check=False, - capture_output=True, - text=True, - timeout=10, - ) - except (OSError, subprocess.TimeoutExpired) as exc: - raise RuntimeError("Failed to read Databricks CLI version.") from exc - - raw = result.stdout or result.stderr or "" - output = (raw if isinstance(raw, str) else raw.decode(errors="replace")).strip() - version = _parse_databricks_cli_version(output) - if version is None: - raise RuntimeError( - f"Could not parse Databricks CLI version from `databricks --version` output: {output!r}" - ) - if version < MIN_DATABRICKS_CLI_VERSION: - current = ".".join(str(n) for n in version) - required = ".".join(str(n) for n in MIN_DATABRICKS_CLI_VERSION) - print_warning( - f"Databricks CLI v{current} is too old (need v{required} or newer). Upgrading..." - ) - _run_databricks_cli_installer(brew_subcommand="upgrade") - ensure_databricks_cli_version() + """Intentionally a no-op: ucode no longer checks or upgrades the installed + Databricks CLI version. + Kept as a seam so callers don't have to care whether a version policy + exists. Managing the CLI is the user's (or their package manager's) job -- + silently replacing a working install, including a locally built one, is + more disruptive than a feature failing with the CLI's own error.""" + return -def install_databricks_cli() -> None: - if shutil.which("databricks"): - ensure_databricks_cli_version() - return - print_section("Bootstrap") - print_warning("`databricks` was not found. Installing Databricks CLI...") - _run_databricks_cli_installer(brew_subcommand="install") +def ensure_databricks_cli() -> None: + """Verify `databricks` is on PATH, raising with install instructions if not. + ucode never installs or upgrades the CLI on the user's behalf.""" if not shutil.which("databricks"): raise RuntimeError( - "Databricks CLI install completed, but `databricks` is still not on PATH." + "Databricks CLI was not found on PATH. Install it, then re-run this command: " + f"see {DATABRICKS_CLI_INSTALL_DOCS_URL} " + "(on macOS: `brew install databricks/tap/databricks`)." ) ensure_databricks_cli_version() @@ -612,9 +556,9 @@ def install_ai_tools(agent_tokens: list[str], profile: str | None = None) -> Non timeout=300, ) except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as exc: - # The CLI version is already guaranteed by ensure_databricks_cli_version, - # so any failure here is something else (e.g. an agent binary missing - # from PATH). Surface the CLI's own error rather than guessing a cause. + # ucode doesn't police the CLI version, so this may be a CLI too old to + # know `aitools` just as easily as something else (e.g. an agent binary + # missing from PATH). Surface the CLI's own error rather than guessing. detail = getattr(exc, "stderr", None) or "" if isinstance(detail, bytes): # TimeoutExpired.stderr is bytes even with text=True detail = detail.decode(errors="replace") diff --git a/tests/test_cli.py b/tests/test_cli.py index eb0cad4..21887cc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -838,7 +838,7 @@ def test_no_extra_args_passes_empty_list(self): class TestConfigureAgentFlag: def test_no_flag_calls_configure_all(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, # Fully-interactive configure ends by offering the MCP step; decline it. @@ -855,7 +855,7 @@ def test_no_flag_calls_configure_all(self): def test_interactive_accepting_mcp_prompt_runs_mcp_config(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command"), patch("ucode.cli.prompt_yes_no", return_value=True), @@ -868,7 +868,7 @@ def test_interactive_accepting_mcp_prompt_runs_mcp_config(self): def test_agents_flag_skips_mcp_prompt(self): # Flag-driven (non-interactive) runs must stay scriptable: no MCP prompt. with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command"), patch("ucode.cli.prompt_yes_no") as mock_prompt, @@ -881,7 +881,7 @@ def test_agents_flag_skips_mcp_prompt(self): def test_agents_flag_calls_configure_with_tools(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary") as mock_install, patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -895,7 +895,7 @@ def test_agents_flag_calls_configure_with_tools(self): def test_agents_flag_normalizes_aliases_and_dedupes(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -908,7 +908,7 @@ def test_agents_flag_normalizes_aliases_and_dedupes(self): def test_workspaces_flag_calls_configure_with_workspaces(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -931,7 +931,7 @@ def test_workspaces_flag_calls_configure_with_workspaces(self): def test_agents_and_workspaces_flags_call_configure_with_both(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -948,7 +948,7 @@ def test_agents_and_workspaces_flags_call_configure_with_both(self): def test_agent_and_workspaces_flags_call_configure_with_both(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary") as mock_install, patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -964,7 +964,7 @@ def test_agent_and_workspaces_flags_call_configure_with_both(self): def test_agent_flag_calls_configure_with_tool(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary") as mock_install, patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -979,7 +979,7 @@ def test_disable_fable_alone_implicitly_targets_claude(self): # Fable is Claude-only, so `--disable-fable` on its own should configure # claude directly instead of dropping into the interactive agent picker. with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary") as mock_install, patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -992,7 +992,7 @@ def test_disable_fable_alone_implicitly_targets_claude(self): def test_enable_fable_alone_implicitly_targets_claude(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary") as mock_install, patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1007,7 +1007,7 @@ def test_enable_fable_with_explicit_agents_does_not_override(self): # An explicit --agents selection wins; the fable flag rides along without # forcing the claude-only single-agent path. with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1021,7 +1021,7 @@ def test_enable_fable_with_explicit_agents_does_not_override(self): def test_skip_upgrade_flag_disables_optional_update_prompt(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, # Fully-interactive configure ends by offering the MCP step; decline it. @@ -1035,7 +1035,7 @@ def test_skip_upgrade_flag_disables_optional_update_prompt(self): def test_disable_databricks_ai_tools_forwards_false_and_skips_prompt(self): # An explicit flag suppresses the interactive prompt and forwards the choice. with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, patch("ucode.cli.prompt_yes_no_default") as mock_prompt, @@ -1052,7 +1052,7 @@ def test_disable_databricks_ai_tools_forwards_false_and_skips_prompt(self): def test_enable_databricks_ai_tools_with_agents_forwards_true(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1113,7 +1113,7 @@ def test_interactive_prompt_defaults_to_prior_optout(self, monkeypatch): def test_skip_upgrade_flag_with_agent_skips_optional_update(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary") as mock_install, patch("ucode.cli.configure_workspace_command"), ): @@ -1125,7 +1125,7 @@ def test_skip_upgrade_flag_with_agent_skips_optional_update(self): def test_skip_upgrade_flag_with_agents_forwards_to_configure(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1138,7 +1138,7 @@ def test_skip_upgrade_flag_with_agents_forwards_to_configure(self): def test_agent_flag_normalizes_alias(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1164,7 +1164,7 @@ def test_upgrade_handles_uv_missing(self): def test_agent_flag_rejects_unknown(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1174,7 +1174,7 @@ def test_agent_flag_rejects_unknown(self): def test_agents_flag_rejects_unknown(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1186,7 +1186,7 @@ def test_agents_flag_rejects_unknown(self): def test_agents_flag_rejects_empty_list(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1196,7 +1196,7 @@ def test_agents_flag_rejects_empty_list(self): def test_agent_and_agents_flags_are_mutually_exclusive(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1206,7 +1206,7 @@ def test_agent_and_agents_flags_are_mutually_exclusive(self): def test_workspaces_flag_rejects_empty_list(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): @@ -1218,7 +1218,7 @@ def test_workspaces_flag_rejects_empty_list(self): class TestConfigureMcpFlag: def test_mcp_with_agents_configures_then_registers_services(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.configure_workspace_command") as mock_cfg, patch("ucode.cli.configure_mcp_command") as mock_mcp, @@ -1238,7 +1238,7 @@ def test_mcp_only_configures_workspace_without_agent_picker(self): # `--mcp` with no --agents (e.g. Cursor): configure the workspace directly, # never the interactive agent picker, then register the MCP service. with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.configure_workspace_command") as mock_cfg, patch("ucode.cli._configure_shared_workspace_states") as mock_shared, patch("ucode.cli.configure_mcp_command") as mock_mcp, @@ -1265,7 +1265,7 @@ def test_mcp_only_configures_workspace_without_agent_picker(self): def test_mcp_rejects_bare_short_name(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.configure_workspace_command"), patch("ucode.cli._configure_shared_workspace_states"), patch("ucode.cli.configure_mcp_command") as mock_mcp, @@ -1475,7 +1475,7 @@ class TestConfigureProfilesFlag: def test_profiles_flag_resolves_workspaces(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.list_profile_entries", return_value=self.PROFILE_ENTRIES), patch("ucode.cli.configure_workspace_command") as mock_cfg, @@ -1491,7 +1491,7 @@ def test_profiles_flag_resolves_workspaces(self): def test_profiles_flag_with_agents(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.list_profile_entries", return_value=self.PROFILE_ENTRIES), patch("ucode.cli.configure_workspace_command") as mock_cfg, @@ -1508,7 +1508,7 @@ def test_profiles_flag_with_agents(self): def test_profiles_flag_with_agent(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.list_profile_entries", return_value=self.PROFILE_ENTRIES), patch("ucode.cli.configure_workspace_command") as mock_cfg, @@ -1522,7 +1522,7 @@ def test_profiles_flag_with_agent(self): def test_use_pat_and_skip_validate_are_forwarded(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.install_tool_binary"), patch("ucode.cli.list_profile_entries", return_value=self.PROFILE_ENTRIES), patch("ucode.cli.configure_workspace_command") as mock_cfg, @@ -1550,7 +1550,7 @@ def test_use_pat_and_skip_validate_are_forwarded(self): def test_use_pat_requires_profiles(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): result = runner.invoke( @@ -1563,7 +1563,7 @@ def test_use_pat_requires_profiles(self): def test_profiles_and_workspaces_are_mutually_exclusive(self): with ( - patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.ensure_databricks_cli"), patch("ucode.cli.configure_workspace_command") as mock_cfg, ): result = runner.invoke( diff --git a/tests/test_databricks.py b/tests/test_databricks.py index 5a3a4ca..fd1a70b 100644 --- a/tests/test_databricks.py +++ b/tests/test_databricks.py @@ -12,8 +12,6 @@ from ucode.databricks import ( AI_GATEWAY_V2_DOCS_URL, _format_subprocess_result, - _parse_databricks_cli_version, - _run_databricks_cli_installer, _scrub_databrickscfg, _scrub_json, build_auth_shell_command, @@ -23,6 +21,7 @@ build_shared_base_urls, build_skills_mcp_url, build_tool_base_url, + ensure_databricks_cli, ensure_databricks_cli_version, ensure_pat_bearer, get_databricks_token, @@ -1791,79 +1790,51 @@ def test_reason_without_body_is_status_only(self): assert reason == "HTTP 404 Not Found" -class TestParseDatabricksCliVersion: - def test_parses_standard_format(self): - assert _parse_databricks_cli_version("Databricks CLI v0.299.2") == (0, 299, 2) - - def test_parses_without_v_prefix(self): - assert _parse_databricks_cli_version("Databricks CLI 0.298.0") == (0, 298, 0) - - def test_returns_none_on_garbage(self): - assert _parse_databricks_cli_version("not a version") is None - - class TestEnsureDatabricksCliVersion: + """The version check is deliberately a no-op: ucode must never inspect or + replace the user's CLI, however old it is.""" + def _fake_databricks(self, tmp_path, version_output: str) -> dict: fake = tmp_path / "databricks" fake.write_text(f"#!/bin/sh\necho '{version_output}'\n") fake.chmod(0o755) return {**os.environ, "PATH": f"{tmp_path}:{os.environ['PATH']}"} - def test_passes_when_version_meets_minimum(self, tmp_path, monkeypatch): - env = self._fake_databricks(tmp_path, "Databricks CLI v1.0.0") - monkeypatch.setattr("os.environ", env) - ensure_databricks_cli_version() # should not raise - - def test_passes_when_version_exceeds_minimum(self, tmp_path, monkeypatch): - env = self._fake_databricks(tmp_path, "Databricks CLI v1.8.0") + @pytest.mark.parametrize( + "version_output", + ["Databricks CLI v1.8.0", "Databricks CLI v0.299.2", "completely broken output"], + ) + def test_never_raises_regardless_of_version(self, tmp_path, monkeypatch, version_output): + env = self._fake_databricks(tmp_path, version_output) monkeypatch.setattr("os.environ", env) ensure_databricks_cli_version() - def test_auto_upgrades_when_version_too_old(self, tmp_path, monkeypatch): - import ucode.databricks as db_mod - - env = self._fake_databricks(tmp_path, "Databricks CLI v0.299.2") - monkeypatch.setattr("os.environ", env) - upgraded = [] + def test_runs_no_subprocess(self, monkeypatch): monkeypatch.setattr( db_mod, - "_run_databricks_cli_installer", - lambda brew_subcommand="install": upgraded.append(brew_subcommand), + "run", + lambda *a, **kw: pytest.fail("ensure_databricks_cli_version must not shell out"), ) - # Stop the recursive re-check after upgrade - call_count = [0] - original = db_mod.ensure_databricks_cli_version - - def once(*a, **kw): - call_count[0] += 1 - if call_count[0] == 1: - original() - - monkeypatch.setattr(db_mod, "ensure_databricks_cli_version", once) - once() - assert upgraded == ["upgrade"] - - def test_raises_when_version_unparseable(self, tmp_path, monkeypatch): - env = self._fake_databricks(tmp_path, "completely broken output") - monkeypatch.setattr("os.environ", env) - with pytest.raises(RuntimeError, match="Could not parse"): - ensure_databricks_cli_version() + ensure_databricks_cli_version() -class TestRunDatabricksCliInstaller: - @pytest.mark.parametrize("brew_subcommand", ["install", "upgrade"]) - def test_macos_uses_fully_qualified_tap_formula(self, monkeypatch, brew_subcommand): - calls = [] - monkeypatch.setattr(db_mod.platform, "system", lambda: "Darwin") - monkeypatch.setattr(db_mod.shutil, "which", lambda cmd: "/opt/homebrew/bin/brew") - monkeypatch.setattr(db_mod, "run", lambda cmd, **kw: calls.append(cmd)) +class TestEnsureDatabricksCli: + def test_passes_when_cli_on_path(self, monkeypatch): + monkeypatch.setattr(db_mod.shutil, "which", lambda cmd: "/usr/local/bin/databricks") + ensure_databricks_cli() - _run_databricks_cli_installer(brew_subcommand=brew_subcommand) + def test_raises_with_install_instructions_when_missing(self, monkeypatch): + monkeypatch.setattr(db_mod.shutil, "which", lambda cmd: None) + with pytest.raises(RuntimeError, match="was not found on PATH"): + ensure_databricks_cli() - # The fully-qualified formula forces Homebrew to the Databricks CLI in - # databricks/tap and fails if absent, rather than falling back to the - # unrelated `databricks` cask. - assert calls == [["brew", brew_subcommand, "databricks/tap/databricks"]] + def test_never_installs_the_cli(self, monkeypatch): + monkeypatch.setattr(db_mod.shutil, "which", lambda cmd: None) + monkeypatch.setattr( + db_mod, "run", lambda *a, **kw: pytest.fail("ucode must not install the CLI") + ) + with pytest.raises(RuntimeError): + ensure_databricks_cli() class TestIsUsageTableAccessError: From a2ce90e9d81b08e5aa137dd1ac6ea6f9204877c0 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 6 Aug 2026 05:38:43 +0000 Subject: [PATCH 2/3] skills: download bundles from the Files API `Skills/` root, not `Volumes/` Skill bundle content is served under `Skills/{cat}/{sch}/{leaf}/...`, so `configure skills` was reading a `Volumes/` path that does not hold the bundle. Both the recursive directory walk and the per-file fetch move over; they have to agree, since relative paths are produced by stripping the prefix off the absolute paths the listing returns. Adds SKILL_FILES_ROOT so the root is stated once, and a test pinning the directory-listing URL -- previously only the file-fetch URL was asserted, so a half-applied change would have gone unnoticed. Co-authored-by: Isaac --- src/ucode/skills_download.py | 20 +++++++++++++------ tests/test_skills_download.py | 37 ++++++++++++++++++++++++----------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/ucode/skills_download.py b/src/ucode/skills_download.py index 124721e..1fd0309 100644 --- a/src/ucode/skills_download.py +++ b/src/ucode/skills_download.py @@ -29,6 +29,11 @@ SKILL_NAME_PATTERN = re.compile(r"^[a-z0-9-]+$") +# Skill bundles are served from the Files API's `Skills/` root, not `Volumes/`. +# Both the directory walk and the per-file fetch must use it, since relative +# paths are derived by stripping this prefix off the listing's absolute paths. +SKILL_FILES_ROOT = "Skills" + # Parallel skill fetches per schema; writes stay sequential (they prompt). _MAX_FETCH_WORKERS = 8 @@ -86,15 +91,15 @@ def list_skill_files( ) -> tuple[list[str], str | None]: """List a skill bundle's files, as paths relative to the skill directory. - Recursively walks the skill's UC Volume directory (including ``SKILL.md``). + Recursively walks the skill's bundle directory (including ``SKILL.md``). A non-None reason indicates the listing call itself failed. """ hostname = workspace_hostname(workspace) dirs_base = f"https://{hostname}/api/2.0/fs/directories" - volume_prefix = f"/Volumes/{catalog}/{schema}/{leaf}/" + bundle_prefix = f"/{SKILL_FILES_ROOT}/{catalog}/{schema}/{leaf}/" relative_paths: list[str] = [] - pending = [f"Volumes/{catalog}/{schema}/{leaf}"] + pending = [f"{SKILL_FILES_ROOT}/{catalog}/{schema}/{leaf}"] while pending: directory = pending.pop() page_token: str | None = None @@ -113,7 +118,7 @@ def list_skill_files( if entry.get("is_directory"): pending.append(path.strip("/")) else: - relative_paths.append(path.removeprefix(volume_prefix)) + relative_paths.append(path.removeprefix(bundle_prefix)) page_token = data.get("next_page_token") if not page_token: break @@ -123,9 +128,12 @@ def list_skill_files( def fetch_skill_file( workspace: str, token: str, catalog: str, schema: str, leaf: str, relative_path: str ) -> tuple[bytes | None, str | None]: - """Fetch one skill bundle file's raw bytes from its UC Volume.""" + """Fetch one skill bundle file's raw bytes from the Files API.""" hostname = workspace_hostname(workspace) - url = f"https://{hostname}/api/2.0/fs/files/Volumes/{catalog}/{schema}/{leaf}/{relative_path}" + url = ( + f"https://{hostname}/api/2.0/fs/files/" + f"{SKILL_FILES_ROOT}/{catalog}/{schema}/{leaf}/{relative_path}" + ) return _http_get_bytes(url, token, timeout=30) diff --git a/tests/test_skills_download.py b/tests/test_skills_download.py index bd5e54d..75f4542 100644 --- a/tests/test_skills_download.py +++ b/tests/test_skills_download.py @@ -97,17 +97,17 @@ def test_http_failure_propagates_reason(self, monkeypatch): class TestListSkillFiles: def test_walks_nested_directories_into_relative_paths(self, monkeypatch): - # The Files API returns absolute `/Volumes/...` paths. - vol = "/Volumes/main/default/triage" + # The Files API returns absolute `/Skills/...` paths. + bundle = "/Skills/main/default/triage" listings = { - "Volumes/main/default/triage": { + "Skills/main/default/triage": { "contents": [ - {"path": f"{vol}/SKILL.md", "is_directory": False}, - {"path": f"{vol}/references/", "is_directory": True}, + {"path": f"{bundle}/SKILL.md", "is_directory": False}, + {"path": f"{bundle}/references/", "is_directory": True}, ] }, - "Volumes/main/default/triage/references": { - "contents": [{"path": f"{vol}/references/primary.md", "is_directory": False}] + "Skills/main/default/triage/references": { + "contents": [{"path": f"{bundle}/references/primary.md", "is_directory": False}] }, } @@ -122,14 +122,29 @@ def fake_get(url, token, timeout=30): assert reason is None assert sorted(paths) == ["SKILL.md", "references/primary.md"] + def test_walks_the_skills_root_not_volumes(self, monkeypatch): + """The listing walk and the per-file fetch must share the `Skills/` + root -- relative paths come from stripping it off the returned paths.""" + captured = {} + + def fake_get(url, token, timeout=30): + captured["url"] = url + return {"contents": []}, None + + monkeypatch.setattr(sd, "_http_get_json", fake_get) + + sd.list_skill_files(WS, "token", "main", "default", "triage") + + assert captured["url"] == f"{WS}/api/2.0/fs/directories/Skills/main/default/triage" + def test_follows_pagination(self, monkeypatch): - vol = "/Volumes/main/default/triage" + bundle = "/Skills/main/default/triage" pages = [ { - "contents": [{"path": f"{vol}/a.md", "is_directory": False}], + "contents": [{"path": f"{bundle}/a.md", "is_directory": False}], "next_page_token": "tok", }, - {"contents": [{"path": f"{vol}/b.md", "is_directory": False}]}, + {"contents": [{"path": f"{bundle}/b.md", "is_directory": False}]}, ] monkeypatch.setattr( @@ -166,7 +181,7 @@ def fake_get_bytes(url, token, timeout=30): assert reason is None assert body == b"# SKILL\n" - assert captured["url"] == f"{WS}/api/2.0/fs/files/Volumes/main/default/triage/SKILL.md" + assert captured["url"] == f"{WS}/api/2.0/fs/files/Skills/main/default/triage/SKILL.md" def test_http_failure_propagates_reason(self, monkeypatch): monkeypatch.setattr( From 7dd43922ac21ddc696259f233f3abfb9cbf7256f Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 6 Aug 2026 06:36:22 +0000 Subject: [PATCH 3/3] Cap mcp below 2.0 so `ucode mcp-proxy` imports `uv tool install` resolves deps fresh and ignores uv.lock, so the unbounded `mcp>=1.28.0` picked up mcp 2.0.0 once it published. That breaks `ucode.mcp_proxy` at import: mcp 2.x moved from httpx to httpx2 and renamed `streamablehttp_client` to `streamable_http_client`, so the proxy died with `ModuleNotFoundError: No module named 'httpx'` and every MCP client saw `-32000: Connection closed`. Lockfile-based runs stayed on 1.28.1, which is why CI never caught it. Only the uv.lock `requires-dist` specifier changes -- the resolved pin is already 1.28.1 and satisfies the cap, so no dependency versions move. Unblocks bugbash. The real follow-up is porting the proxy to mcp 2.x. Co-authored-by: Isaac --- pyproject.toml | 5 ++++- uv.lock | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 781bf1d..dff5a8e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,7 +13,10 @@ dependencies = [ # `ucode mcp-proxy` bridges a client's stdio MCP transport to a Databricks # streamable-HTTP MCP endpoint, injecting a freshly-minted OAuth bearer per # request. Uses the official MCP SDK's stdio server + streamable-HTTP client. - "mcp>=1.28.0", + # Capped below 2.0: mcp 2.x swaps httpx for httpx2 and renames + # `streamablehttp_client` to `streamable_http_client`, so `ucode.mcp_proxy` + # fails to import against it. Lift once the proxy is ported. + "mcp>=1.28.0,<2", "questionary>=2.0.0", "tomlkit>=0.13.0", "typer>=0.12.0", diff --git a/uv.lock b/uv.lock index bb2e29a..b18c0b5 100644 --- a/uv.lock +++ b/uv.lock @@ -3180,7 +3180,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "databricks-sql-connector", specifier = ">=3.6.0" }, - { name = "mcp", specifier = ">=1.28.0" }, + { name = "mcp", specifier = ">=1.28.0,<2" }, { name = "mlflow", extras = ["databricks"], marker = "extra == 'tracing'", specifier = ">=3.4" }, { name = "questionary", specifier = ">=2.0.0" }, { name = "tomlkit", specifier = ">=0.13.0" },