From 934871a7fcf982f0cc3423775a44909f58b70eac Mon Sep 17 00:00:00 2001 From: scottDBX1886 Date: Mon, 13 Apr 2026 09:22:56 -0400 Subject: [PATCH] updated the skills_manager to support both local and worksapce deployments. also removed a hash requirement in the requitements.txt --- databricks-builder-app/requirements.txt | 4 +- .../server/services/skills_manager.py | 130 +++++++++++------- 2 files changed, 83 insertions(+), 51 deletions(-) diff --git a/databricks-builder-app/requirements.txt b/databricks-builder-app/requirements.txt index 0c6d6e2b..4bba6c3b 100644 --- a/databricks-builder-app/requirements.txt +++ b/databricks-builder-app/requirements.txt @@ -352,9 +352,7 @@ pyjwt==2.12.1 # via mcp pyparsing==3.3.2 # via matplotlib -pyperclip==1.11.0 \ - --hash=sha256:244035963e4428530d9e3a6101a1ef97209c6825edab1567beac148ccc1db1b6 \ - --hash=sha256:299403e9ff44581cb9ba2ffeed69c7aa96a008622ad0c46cb575ca75b5b84273 +pyperclip==1.11.0 # via fastmcp pytest==9.0.2 # via sqlfluff diff --git a/databricks-builder-app/server/services/skills_manager.py b/databricks-builder-app/server/services/skills_manager.py index 7f91542f..fccadc54 100644 --- a/databricks-builder-app/server/services/skills_manager.py +++ b/databricks-builder-app/server/services/skills_manager.py @@ -95,25 +95,40 @@ def get_allowed_mcp_tools( ] -# Source skills directory - check multiple locations -# 1. Sibling to this app (local development): ../../databricks-skills -# 2. Deployed location (Databricks Apps): ./skills at app root -_DEV_SKILLS_DIR = Path(__file__).parent.parent.parent.parent / 'databricks-skills' -_DEPLOYED_SKILLS_DIR = Path(__file__).parent.parent.parent / 'skills' +# Skills source directories. install_skills.sh aggregates skills from +# multiple repos (this repo's databricks-skills/, mlflow/skills, apx) into +# the app's .claude/skills/ directory. We check several locations so that +# the server works both in local development and when deployed. +# +# Candidate source directories (checked in priority order): +# 1. .claude/skills/ inside the app — populated by install_skills.sh with +# the *full* union of Databricks + MLflow + APX skills. +# 2. Sibling ../../databricks-skills — the repo-local Databricks-only skills. +# 3. ./skills at app root — the deployed bundle location. +_APP_ROOT = Path(__file__).parent.parent.parent +_INSTALLED_SKILLS_DIR = _APP_ROOT / '.claude' / 'skills' +_DEV_SKILLS_DIR = _APP_ROOT.parent / 'databricks-skills' +_DEPLOYED_SKILLS_DIR = _APP_ROOT / 'skills' # Local cache of skills within this app (copied on startup) -APP_SKILLS_DIR = Path(__file__).parent.parent.parent / 'skills' +APP_SKILLS_DIR = _APP_ROOT / 'skills' -# Prefer dev location (sibling repo) when available to avoid self-deletion: -# APP_SKILLS_DIR == _DEPLOYED_SKILLS_DIR, so using deployed as source would -# delete the source during copy_skills_to_app(). Only fall back to deployed -# location when the dev source repo isn't available (actual deployment). -if _DEV_SKILLS_DIR.exists() and any(_DEV_SKILLS_DIR.iterdir()): - SKILLS_SOURCE_DIR = _DEV_SKILLS_DIR -elif _DEPLOYED_SKILLS_DIR.exists() and any(_DEPLOYED_SKILLS_DIR.iterdir()): - SKILLS_SOURCE_DIR = _DEPLOYED_SKILLS_DIR -else: - SKILLS_SOURCE_DIR = _DEV_SKILLS_DIR +def _non_empty_dir(p: Path) -> bool: + return p.exists() and p.is_dir() and any(p.iterdir()) + +# Build an ordered list of source directories. The first directory that +# contains a given skill wins, so put the most-complete source first. +_SKILLS_SOURCE_DIRS: list[Path] = [] +if _non_empty_dir(_INSTALLED_SKILLS_DIR): + _SKILLS_SOURCE_DIRS.append(_INSTALLED_SKILLS_DIR) +if _non_empty_dir(_DEV_SKILLS_DIR): + _SKILLS_SOURCE_DIRS.append(_DEV_SKILLS_DIR) +if _non_empty_dir(_DEPLOYED_SKILLS_DIR) and _DEPLOYED_SKILLS_DIR.resolve() != APP_SKILLS_DIR.resolve(): + _SKILLS_SOURCE_DIRS.append(_DEPLOYED_SKILLS_DIR) + +# Legacy single-directory reference used by callers that haven't been +# updated yet. Points to the first available source. +SKILLS_SOURCE_DIR = _SKILLS_SOURCE_DIRS[0] if _SKILLS_SOURCE_DIRS else _DEV_SKILLS_DIR def _get_enabled_skills() -> list[str] | None: @@ -190,8 +205,24 @@ class SkillNotFoundError(Exception): pass +def _find_skill_source(skill_name: str) -> Path | None: + """Find a skill directory across all source directories. + + Returns the first directory that contains ``skill_name/SKILL.md``. + """ + for src_dir in _SKILLS_SOURCE_DIRS: + candidate = src_dir / skill_name + if candidate.is_dir() and (candidate / 'SKILL.md').exists(): + return candidate + return None + + def copy_skills_to_app() -> bool: - """Copy skills from source repo to app's skills directory. + """Copy skills from source directories to app's skills directory. + + Skills may originate from multiple locations (the repo's databricks-skills/, + the .claude/skills/ directory populated by install_skills.sh, or the + deployed skills bundle). This function merges them into APP_SKILLS_DIR. Called on server startup to ensure we have the latest skills. Only copies skills listed in ENABLED_SKILLS env var (if set). @@ -200,68 +231,71 @@ def copy_skills_to_app() -> bool: True if successful, False otherwise Raises: - SkillNotFoundError: If an enabled skill folder doesn't exist or lacks SKILL.md + SkillNotFoundError: If an enabled skill folder doesn't exist in any + source directory or lacks SKILL.md. """ - if not SKILLS_SOURCE_DIR.exists(): - logger.warning(f'Skills source directory not found: {SKILLS_SOURCE_DIR}') + if not _SKILLS_SOURCE_DIRS: + # No external source directories found. In a deployed app the skills + # are bundled directly into APP_SKILLS_DIR (== _DEPLOYED_SKILLS_DIR), + # so there is nothing to copy — skills are already in place. + if _non_empty_dir(APP_SKILLS_DIR): + logger.info(f'No external source dirs; skills already in place at {APP_SKILLS_DIR}') + return True + logger.warning('No skills source directories found') return False - # Guard against self-deletion: in deployed apps, SKILLS_SOURCE_DIR and - # APP_SKILLS_DIR resolve to the same directory. Deleting APP_SKILLS_DIR - # would destroy the source. Skills are already in place, so skip the copy. - if SKILLS_SOURCE_DIR.resolve() == APP_SKILLS_DIR.resolve(): - logger.info(f'Skills source and app directory are the same ({APP_SKILLS_DIR}), skipping copy') + # Guard against self-deletion: when every source *is* APP_SKILLS_DIR we + # would wipe the only copy. Skills are already in place, so skip. + all_same = all( + src.resolve() == APP_SKILLS_DIR.resolve() for src in _SKILLS_SOURCE_DIRS + ) + if all_same: + logger.info(f'All skills sources resolve to {APP_SKILLS_DIR}, skipping copy') return True enabled_skills = _get_enabled_skills() if enabled_skills: logger.info(f'Filtering skills to: {enabled_skills}') - # Validate that all enabled skills exist before copying for skill_name in enabled_skills: - skill_path = SKILLS_SOURCE_DIR / skill_name - skill_md_path = skill_path / 'SKILL.md' - - if not skill_path.exists(): + found = _find_skill_source(skill_name) + if found is None: + searched = ', '.join(str(d) for d in _SKILLS_SOURCE_DIRS) raise SkillNotFoundError( - f"Skill '{skill_name}' not found. " - f"Directory does not exist: {skill_path}. " + f"Skill '{skill_name}' not found in any source directory. " + f"Searched: {searched}. " f"Check ENABLED_SKILLS in your .env file." ) - if not skill_md_path.exists(): - raise SkillNotFoundError( - f"Skill '{skill_name}' is invalid. " - f"Missing SKILL.md file in: {skill_path}. " - f"Each skill must have a SKILL.md file." - ) - try: - # Remove existing skills directory if it exists if APP_SKILLS_DIR.exists(): shutil.rmtree(APP_SKILLS_DIR) - # Copy skill directories (filtered by ENABLED_SKILLS if set) APP_SKILLS_DIR.mkdir(parents=True, exist_ok=True) - copied_count = 0 - for item in SKILLS_SOURCE_DIR.iterdir(): - if item.is_dir() and (item / 'SKILL.md').exists(): - # Skip if not in enabled list (when list is specified) + copied: set[str] = set() + for src_dir in _SKILLS_SOURCE_DIRS: + if src_dir.resolve() == APP_SKILLS_DIR.resolve(): + continue + for item in src_dir.iterdir(): + if not item.is_dir() or not (item / 'SKILL.md').exists(): + continue + if item.name in copied: + continue if enabled_skills and item.name not in enabled_skills: logger.debug(f'Skipping skill (not enabled): {item.name}') continue dest = APP_SKILLS_DIR / item.name shutil.copytree(item, dest) - copied_count += 1 + copied.add(item.name) logger.debug(f'Copied skill: {item.name}') - logger.info(f'Copied {copied_count} skills to {APP_SKILLS_DIR}') + logger.info(f'Copied {len(copied)} skills to {APP_SKILLS_DIR}') return True except SkillNotFoundError: - raise # Re-raise validation errors + raise except Exception as e: logger.error(f'Failed to copy skills: {e}') return False