From 3ebe1676d6acae645418cc346cd2b1a24e55f411 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 23 Jun 2026 20:38:43 -0700 Subject: [PATCH] [eric] skills+registries: leading-_ -> p_/public across skills/skill_registry/mcp_registry; promote cross-file public (sync_skills/select_skill_paths/etc.), fix qualified test refs --- .../agents/manager/prompt/prompt_context.py | 4 +- backend/apps/mcp_registry/mcp_registry.py | 94 ++++++------- backend/apps/skill_registry/skill_registry.py | 132 +++++++++--------- backend/apps/skills/skills.py | 82 +++++------ .../tests/test_skill_registry_community.py | 36 ++--- 5 files changed, 174 insertions(+), 174 deletions(-) diff --git a/backend/apps/agents/manager/prompt/prompt_context.py b/backend/apps/agents/manager/prompt/prompt_context.py index eacb80da..42c011fd 100644 --- a/backend/apps/agents/manager/prompt/prompt_context.py +++ b/backend/apps/agents/manager/prompt/prompt_context.py @@ -402,8 +402,8 @@ def resolve_attached_skills(attached_skills: Optional[List]) -> str: return "" folder_by_id: Dict[str, str] = {} try: - from backend.apps.skills.skills import _sync_skills - for s in _sync_skills(): + from backend.apps.skills.skills import sync_skills + for s in sync_skills(): if s.dir_path and s.has_supporting_files: folder_by_id[s.id] = s.dir_path except Exception: diff --git a/backend/apps/mcp_registry/mcp_registry.py b/backend/apps/mcp_registry/mcp_registry.py index 4a8bdeac..f3b35a5c 100644 --- a/backend/apps/mcp_registry/mcp_registry.py +++ b/backend/apps/mcp_registry/mcp_registry.py @@ -20,13 +20,13 @@ GITHUB_TOKEN = os.environ.get("GITHUB_TOKEN", "") GITHUB_BATCH = 4000 if GITHUB_TOKEN else 50 GITHUB_CONCURRENT = 10 -_cache: dict[str, dict] = {} -_cache_updated_at: float = 0 -_refresh_task: Optional[asyncio.Task] = None -_stars_cache: dict[str, int] = {} +p_cache: dict[str, dict] = {} +p_cache_updated_at: float = 0 +p_refresh_task: Optional[asyncio.Task] = None +p_stars_cache: dict[str, int] = {} -def _extract_gh_repo(repo_url: str) -> Optional[str]: +def p_extract_gh_repo(repo_url: str) -> Optional[str]: """Parse 'owner/repo' from a GitHub URL.""" if not repo_url or "github.com" not in repo_url: return None @@ -42,7 +42,7 @@ def _extract_gh_repo(repo_url: str) -> Optional[str]: return None -def _extract_server(entry: dict) -> Optional[dict]: +def p_extract_server(entry: dict) -> Optional[dict]: """Extract a flat server record from a registry entry, keeping only latest versions.""" meta = entry.get("_meta", {}).get("io.modelcontextprotocol.registry/official", {}) if not meta.get("isLatest"): @@ -96,7 +96,7 @@ def _extract_server(entry: dict) -> Optional[dict]: } -async def _fetch_all_servers() -> dict[str, dict]: +async def p_fetch_all_servers() -> dict[str, dict]: """Paginate through the full registry and return a dict keyed by server name.""" servers: dict[str, dict] = {} cursor: Optional[str] = None @@ -121,7 +121,7 @@ async def _fetch_all_servers() -> dict[str, dict]: break for entry in entries: - record = _extract_server(entry) + record = p_extract_server(entry) if record: servers[record["name"]] = record @@ -137,14 +137,14 @@ async def _fetch_all_servers() -> dict[str, dict]: GOOGLE_README_URL = "https://raw.githubusercontent.com/google/mcp/main/README.md" GOOGLE_ICON_URL = "https://github.com/google.png?size=64" -_ENTRY_RE = re.compile(r"\[\*\*(.+?)\*\*\]\((.+?)\)(?:[,\s]*(.+))?") +P_ENTRY_RE = re.compile(r"\[\*\*(.+?)\*\*\]\((.+?)\)(?:[,\s]*(.+))?") -def _slugify(name: str) -> str: +def p_slugify(name: str) -> str: return re.sub(r"[^a-z0-9]+", "-", name.lower()).strip("-") -def _parse_google_readme(text: str) -> dict[str, dict]: +def p_parse_google_readme(text: str) -> dict[str, dict]: servers: dict[str, dict] = {} section: Optional[str] = None @@ -164,7 +164,7 @@ def _parse_google_readme(text: str) -> dict[str, dict]: if section is None: continue - m = _ENTRY_RE.search(stripped) + m = P_ENTRY_RE.search(stripped) if not m: continue @@ -172,7 +172,7 @@ def _parse_google_readme(text: str) -> dict[str, dict]: url = m.group(2).strip() desc_raw = (m.group(3) or "").strip().rstrip(".") - slug = _slugify(title) + slug = p_slugify(title) key = f"google/{slug}" is_github = "github.com" in url or "go.dev" in url @@ -206,13 +206,13 @@ def _parse_google_readme(text: str) -> dict[str, dict]: return servers -async def _fetch_google_servers() -> dict[str, dict]: +async def p_fetch_google_servers() -> dict[str, dict]: """Fetch and parse Google's MCP server catalog from their GitHub README.""" try: async with httpx.AsyncClient(timeout=15.0) as client: resp = await client.get(GOOGLE_README_URL) resp.raise_for_status() - servers = _parse_google_readme(resp.text) + servers = p_parse_google_readme(resp.text) logger.info(f"Google MCP catalog: parsed {len(servers)} servers") return servers except Exception as e: @@ -220,29 +220,29 @@ async def _fetch_google_servers() -> dict[str, dict]: return {} -async def _fetch_github_stars(servers: dict[str, dict]): +async def p_fetch_github_stars(servers: dict[str, dict]): """Batch-fetch GitHub star counts for servers with GitHub repos. Uses an in-memory cache so stars accumulate across refresh cycles even when rate-limited (60 req/hr unauthenticated, 5 000 with GITHUB_TOKEN). """ - global _stars_cache + global p_stars_cache needed: list[str] = [] for srv in servers.values(): - gh = _extract_gh_repo(srv.get("repositoryUrl", "")) - if gh and gh not in _stars_cache and gh not in needed: + gh = p_extract_gh_repo(srv.get("repositoryUrl", "")) + if gh and gh not in p_stars_cache and gh not in needed: needed.append(gh) if not needed: - logger.info(f"GitHub stars: all {len(_stars_cache)} repos cached, 0 to fetch") - _apply_stars(servers) + logger.info(f"GitHub stars: all {len(p_stars_cache)} repos cached, 0 to fetch") + p_apply_stars(servers) return to_fetch = needed[: GITHUB_BATCH] logger.info( f"GitHub stars: fetching {len(to_fetch)} repos " - f"({len(_stars_cache)} cached, {len(needed)} pending)" + f"({len(p_stars_cache)} cached, {len(needed)} pending)" ) headers: dict[str, str] = {"Accept": "application/vnd.github.v3+json"} @@ -265,13 +265,13 @@ async def _fetch_github_stars(servers: dict[str, dict]): f"https://api.github.com/repos/{repo}", headers=headers ) if resp.status_code == 200: - _stars_cache[repo] = resp.json().get("stargazers_count", 0) + p_stars_cache[repo] = resp.json().get("stargazers_count", 0) fetched += 1 elif resp.status_code in (403, 429): rate_limited = True logger.warning("GitHub API rate-limited, stopping star fetch") elif resp.status_code == 404: - _stars_cache[repo] = 0 + p_stars_cache[repo] = 0 fetched += 1 except Exception as exc: logger.debug(f"GitHub stars fetch failed for {repo}: {exc}") @@ -279,28 +279,28 @@ async def _fetch_github_stars(servers: dict[str, dict]): async with httpx.AsyncClient(timeout=15.0) as client: await asyncio.gather(*[_fetch_one(client, r) for r in to_fetch]) - logger.info(f"GitHub stars: fetched {fetched} new, {len(_stars_cache)} total cached") - _apply_stars(servers) + logger.info(f"GitHub stars: fetched {fetched} new, {len(p_stars_cache)} total cached") + p_apply_stars(servers) -def _apply_stars(servers: dict[str, dict]): +def p_apply_stars(servers: dict[str, dict]): for srv in servers.values(): - gh = _extract_gh_repo(srv.get("repositoryUrl", "")) - srv["stars"] = _stars_cache.get(gh) if gh else None + gh = p_extract_gh_repo(srv.get("repositoryUrl", "")) + srv["stars"] = p_stars_cache.get(gh) if gh else None -async def _refresh_loop(): +async def p_refresh_loop(): """Background loop that refreshes the cache on startup and then hourly.""" - global _cache, _cache_updated_at + global p_cache, p_cache_updated_at while True: try: community, google = await asyncio.gather( - _fetch_all_servers(), - _fetch_google_servers(), + p_fetch_all_servers(), + p_fetch_google_servers(), ) - _cache = {**community, **google} - await _fetch_github_stars(_cache) - _cache_updated_at = time.time() + p_cache = {**community, **google} + await p_fetch_github_stars(p_cache) + p_cache_updated_at = time.time() except Exception as e: logger.exception(f"MCP registry refresh error: {e}") await asyncio.sleep(REFRESH_INTERVAL_S) @@ -308,13 +308,13 @@ async def _refresh_loop(): @asynccontextmanager async def mcp_registry_lifespan(): - global _refresh_task - _refresh_task = asyncio.create_task(_refresh_loop()) + global p_refresh_task + p_refresh_task = asyncio.create_task(p_refresh_loop()) yield - if _refresh_task: - _refresh_task.cancel() + if p_refresh_task: + p_refresh_task.cancel() try: - await _refresh_task + await p_refresh_task except asyncio.CancelledError: pass @@ -324,13 +324,13 @@ mcp_registry = SubApp("mcp-registry", mcp_registry_lifespan) @mcp_registry.router.get("/stats") async def registry_stats(): - google = sum(1 for s in _cache.values() if s.get("source") == "google") - community = sum(1 for s in _cache.values() if s.get("source") == "community") + google = sum(1 for s in p_cache.values() if s.get("source") == "google") + community = sum(1 for s in p_cache.values() if s.get("source") == "community") return { - "total": len(_cache), + "total": len(p_cache), "google": google, "community": community, - "lastUpdated": _cache_updated_at, + "lastUpdated": p_cache_updated_at, } @@ -342,7 +342,7 @@ async def registry_search( sort: str = Query("name", description="Sort by: name, stars"), source: str = Query("", description="Filter by source: google, community, or empty for all"), ): - pool = _cache.values() + pool = p_cache.values() if source: pool = [s for s in pool if s.get("source") == source] @@ -387,7 +387,7 @@ async def registry_search( @mcp_registry.router.get("/detail/{server_name:path}") async def registry_detail(server_name: str): - srv = _cache.get(server_name) + srv = p_cache.get(server_name) if not srv: return {"error": "Server not found"}, 404 return {"server": srv} diff --git a/backend/apps/skill_registry/skill_registry.py b/backend/apps/skill_registry/skill_registry.py index a496ae91..30dba42a 100644 --- a/backend/apps/skill_registry/skill_registry.py +++ b/backend/apps/skill_registry/skill_registry.py @@ -23,32 +23,32 @@ CONCURRENT_FETCHES = 15 # Retry the startup fetch on this short backoff (capped) until the FIRST success, # instead of waiting a full REFRESH_INTERVAL_S after a cold/slow/failed fetch. # That 1h gap was the "skills empty until reboot" bug on cold Windows networks. -_RETRY_BACKOFF_START_S = 2 -_RETRY_BACKOFF_MAX_S = 60 +P_RETRY_BACKOFF_START_S = 2 +P_RETRY_BACKOFF_MAX_S = 60 # Catalog ships in the repo so a brand-new install shows skills with zero network # (build snapshot), and every successful live fetch is persisted to the user's # cache so subsequent launches are instant + offline-safe. The live fetch always # overwrites both once it lands, so neither can go stale at runtime. -_BUNDLED_SNAPSHOT = os.path.join(os.path.dirname(__file__), "skills_snapshot.json") +P_BUNDLED_SNAPSHOT = os.path.join(os.path.dirname(__file__), "skills_snapshot.json") -_cache: dict[str, dict] = {} -_cache_updated_at: float = 0 -_refresh_task: Optional[asyncio.Task] = None +p_cache: dict[str, dict] = {} +p_cache_updated_at: float = 0 +p_refresh_task: Optional[asyncio.Task] = None -def _disk_cache_path() -> str: +def p_disk_cache_path() -> str: base = os.environ.get("OPENSWARM_SKILL_CACHE_DIR") or os.path.expanduser( "~/.openswarm/cache" ) return os.path.join(base, "skill_registry.json") -def _load_seed_cache() -> dict[str, dict]: +def p_load_seed_cache() -> dict[str, dict]: """Return a non-empty catalog from the on-disk last-good cache, falling back to the bundled snapshot, so the registry is never empty on a cold/offline start. Returns {} only if neither source is present/valid.""" - for path in (_disk_cache_path(), _BUNDLED_SNAPSHOT): + for path in (p_disk_cache_path(), P_BUNDLED_SNAPSHOT): try: with open(path, encoding="utf-8") as f: data = json.load(f) @@ -60,12 +60,12 @@ def _load_seed_cache() -> dict[str, dict]: return {} -def _save_disk_cache(skills: dict[str, dict]) -> None: +def p_save_disk_cache(skills: dict[str, dict]) -> None: """Persist the last good live fetch so the next launch is instant. Atomic replace so a crash mid-write can't leave a truncated cache.""" if not skills: return - path = _disk_cache_path() + path = p_disk_cache_path() try: os.makedirs(os.path.dirname(path), exist_ok=True) tmp = f"{path}.tmp" @@ -76,7 +76,7 @@ def _save_disk_cache(skills: dict[str, dict]) -> None: logger.debug("Skill registry: could not persist disk cache", exc_info=True) -def _parse_frontmatter(raw: str) -> tuple[dict, str]: +def p_parse_frontmatter(raw: str) -> tuple[dict, str]: """Split YAML frontmatter from markdown body.""" if not raw.startswith("---"): return {}, raw @@ -93,7 +93,7 @@ def _parse_frontmatter(raw: str) -> tuple[dict, str]: return meta, body -async def _fetch_skill_paths(client: httpx.AsyncClient) -> list[tuple[str, str]]: +async def p_fetch_skill_paths(client: httpx.AsyncClient) -> list[tuple[str, str]]: """Fetch the marketplace.json manifest and return (skill_folder, plugin_name) pairs. Uses raw.githubusercontent.com; no GitHub API needed, no rate limiting. @@ -111,7 +111,7 @@ async def _fetch_skill_paths(client: httpx.AsyncClient) -> list[tuple[str, str]] return paths -async def _fetch_one_skill( +async def p_fetch_one_skill( client: httpx.AsyncClient, sem: asyncio.Semaphore, folder: str, @@ -127,7 +127,7 @@ async def _fetch_one_skill( logger.debug(f"Failed to fetch {folder}/SKILL.md: {exc}") return None - meta, body = _parse_frontmatter(raw) + meta, body = p_parse_frontmatter(raw) name = meta.get("name", "") if not name: folder_name = folder.rsplit("/", 1)[-1] @@ -143,11 +143,11 @@ async def _fetch_one_skill( } -async def _fetch_all_skills() -> dict[str, dict]: +async def p_fetch_all_skills() -> dict[str, dict]: skills: dict[str, dict] = {} async with httpx.AsyncClient(timeout=30.0) as client: try: - paths = await _fetch_skill_paths(client) + paths = await p_fetch_skill_paths(client) except Exception as e: logger.warning(f"Skill registry manifest fetch failed: {e}") return skills @@ -155,7 +155,7 @@ async def _fetch_all_skills() -> dict[str, dict]: logger.info(f"Skill registry: found {len(paths)} skills in manifest, fetching content...") sem = asyncio.Semaphore(CONCURRENT_FETCHES) results = await asyncio.gather( - *[_fetch_one_skill(client, sem, folder, plugin) for folder, plugin in paths] + *[p_fetch_one_skill(client, sem, folder, plugin) for folder, plugin in paths] ) for rec in results: if rec: @@ -165,45 +165,45 @@ async def _fetch_all_skills() -> dict[str, dict]: return skills -async def _refresh_loop(): - global _cache, _cache_updated_at - backoff = _RETRY_BACKOFF_START_S +async def p_refresh_loop(): + global p_cache, p_cache_updated_at + backoff = P_RETRY_BACKOFF_START_S while True: ok = False try: - fetched = await _fetch_all_skills() + fetched = await p_fetch_all_skills() if fetched: - _cache = fetched - _cache_updated_at = time.time() - _save_disk_cache(_cache) + p_cache = fetched + p_cache_updated_at = time.time() + p_save_disk_cache(p_cache) ok = True except Exception as e: logger.exception(f"Skill registry refresh error: {e}") if ok: # Settle to the slow hourly refresh once we have a good catalog. - backoff = _RETRY_BACKOFF_START_S + backoff = P_RETRY_BACKOFF_START_S await asyncio.sleep(REFRESH_INTERVAL_S) else: # Cold/slow/failed fetch: retry soon (capped) until the first success # so a transient network hiccup doesn't leave the catalog empty for # an hour. The seeded snapshot keeps it non-empty meanwhile. await asyncio.sleep(backoff) - backoff = min(backoff * 2, _RETRY_BACKOFF_MAX_S) + backoff = min(backoff * 2, P_RETRY_BACKOFF_MAX_S) @asynccontextmanager async def skill_registry_lifespan(): - global _refresh_task, _cache + global p_refresh_task, p_cache # Seed instantly from disk/bundled snapshot so the very first request never # sees an empty catalog (the live fetch below overwrites it when it lands). - if not _cache: - _cache = _load_seed_cache() - _refresh_task = asyncio.create_task(_refresh_loop()) + if not p_cache: + p_cache = p_load_seed_cache() + p_refresh_task = asyncio.create_task(p_refresh_loop()) yield - if _refresh_task: - _refresh_task.cancel() + if p_refresh_task: + p_refresh_task.cancel() try: - await _refresh_task + await p_refresh_task except asyncio.CancelledError: pass @@ -214,13 +214,13 @@ skill_registry = SubApp("skill-registry", skill_registry_lifespan) @skill_registry.router.get("/stats") async def registry_stats(): categories: dict[str, int] = {} - for s in _cache.values(): + for s in p_cache.values(): cat = s.get("category", "General") categories[cat] = categories.get(cat, 0) + 1 return { - "total": len(_cache), + "total": len(p_cache), "categories": categories, - "lastUpdated": _cache_updated_at, + "lastUpdated": p_cache_updated_at, } @@ -236,12 +236,12 @@ async def registry_search( # The wild registry is a remote 600k-entry index, searched live, not mirrored. if source == "community": try: - return await _community_search(q, limit) + return await p_community_search(q, limit) except Exception as e: logger.warning(f"community skill search failed: {e}") return {"skills": [], "total": 0, "offset": 0, "limit": limit, "source": "community", "error": "skills.sh unreachable"} - pool = list(_cache.values()) + pool = list(p_cache.values()) if category: cat_lower = category.lower() pool = [s for s in pool if s.get("category", "").lower() == cat_lower] @@ -274,7 +274,7 @@ async def registry_search( @skill_registry.router.get("/detail/{skill_name:path}") async def registry_detail(skill_name: str): - sk = _cache.get(skill_name) + sk = p_cache.get(skill_name) if not sk: return {"error": "Skill not found"}, 404 return {"skill": sk} @@ -289,22 +289,22 @@ async def registry_detail(skill_name: str): # and any skill script later runs through the same gated Bash path as anything. # --------------------------------------------------------------------------- -_COMMUNITY_SEARCH_URL = "https://skills.sh/api/search" -_GH_API = "https://api.github.com" -_GH_RAW = "https://raw.githubusercontent.com" -_MAX_SKILL_FILES = 60 -_SCRIPT_EXTS = (".sh", ".py", ".js", ".mjs", ".cjs", ".ts", ".rb", ".pl", ".ps1", ".bat", ".php") +P_COMMUNITY_SEARCH_URL = "https://skills.sh/api/search" +P_GH_API = "https://api.github.com" +P_GH_RAW = "https://raw.githubusercontent.com" +P_MAX_SKILL_FILES = 60 +P_SCRIPT_EXTS = (".sh", ".py", ".js", ".mjs", ".cjs", ".ts", ".rb", ".pl", ".ps1", ".bat", ".php") -def _is_script_path(rel: str) -> bool: +def is_script_path(rel: str) -> bool: """Whether a skill file is executable code worth disclosing before install.""" - if rel.lower().endswith(_SCRIPT_EXTS): + if rel.lower().endswith(P_SCRIPT_EXTS): return True head = rel.split("/", 1)[0].lower() return head in ("scripts", "bin", "hooks") -def _github_headers() -> dict: +def github_headers() -> dict: """GitHub request headers, with auth if a token is set. Unauthenticated is 60 req/hr/IP (fine for the odd install, the wall for a power user); a token (OPENSWARM_GITHUB_TOKEN or GITHUB_TOKEN) raises it to 5000/hr.""" @@ -315,7 +315,7 @@ def _github_headers() -> dict: return headers -def _select_skill_paths(tree: list[dict], skill_id: str) -> tuple[str, list[str]]: +def select_skill_paths(tree: list[dict], skill_id: str) -> tuple[str, list[str]]: """From a GitHub recursive tree, pick the SKILL.md for `skill_id` and every file beside it. Pure, so the resolution logic is unit-tested without a network round-trip. When a repo has several `//SKILL.md` matches the pick @@ -337,7 +337,7 @@ def _select_skill_paths(tree: list[dict], skill_id: str) -> tuple[str, list[str] skill_dir = skill_md[: -len("/SKILL.md")] if "/" in skill_md else "" prefix = (skill_dir + "/") if skill_dir else "" members = [p for p in blobs if (p.startswith(prefix) if prefix else "/" not in p)] - return skill_md, members[:_MAX_SKILL_FILES] + return skill_md, members[:P_MAX_SKILL_FILES] class RegistryRateLimited(Exception): @@ -345,9 +345,9 @@ class RegistryRateLimited(Exception): 'try again shortly' rather than a generic failure.""" -async def _tree_at(client: httpx.AsyncClient, owner: str, repo: str, branch: str): +async def p_tree_at(client: httpx.AsyncClient, owner: str, repo: str, branch: str): """(tree | None) for a branch. None on 404 (branch absent); raises on 403.""" - r = await client.get(f"{_GH_API}/repos/{owner}/{repo}/git/trees/{branch}?recursive=1") + r = await client.get(f"{P_GH_API}/repos/{owner}/{repo}/git/trees/{branch}?recursive=1") if r.status_code == 200: return r.json().get("tree", []) if r.status_code == 403: @@ -355,22 +355,22 @@ async def _tree_at(client: httpx.AsyncClient, owner: str, repo: str, branch: str return None -async def _fetch_repo_tree(client: httpx.AsyncClient, owner: str, repo: str) -> tuple[str, list[dict]]: +async def p_fetch_repo_tree(client: httpx.AsyncClient, owner: str, repo: str) -> tuple[str, list[dict]]: """Recursive tree of owner/repo. Tries main then master first (one call, the 99% case, no quota wasted on a repo-meta lookup); only if BOTH are absent does it ask the repo for its real default branch (handles develop/trunk/etc). Raises RegistryRateLimited on a 403, ValueError if no branch resolves.""" for branch in ("main", "master"): - tree = await _tree_at(client, owner, repo, branch) + tree = await p_tree_at(client, owner, repo, branch) if tree is not None: return branch, tree - meta = await client.get(f"{_GH_API}/repos/{owner}/{repo}") + meta = await client.get(f"{P_GH_API}/repos/{owner}/{repo}") if meta.status_code == 403: raise RegistryRateLimited() if meta.status_code == 200: default = meta.json().get("default_branch") if default and default not in ("main", "master"): - tree = await _tree_at(client, owner, repo, default) + tree = await p_tree_at(client, owner, repo, default) if tree is not None: return default, tree raise ValueError(f"repo {owner}/{repo} has no resolvable default branch") @@ -385,22 +385,22 @@ async def resolve_community_skill(source: str, skill_id: str) -> dict: owner, _, repo = source.partition("/") if not owner or not repo: raise ValueError(f"unrecognized source '{source}' (expected owner/repo)") - async with httpx.AsyncClient(timeout=30.0, headers=_github_headers()) as client: - branch, tree = await _fetch_repo_tree(client, owner, repo) - skill_md, members = _select_skill_paths(tree, skill_id) + async with httpx.AsyncClient(timeout=30.0, headers=github_headers()) as client: + branch, tree = await p_fetch_repo_tree(client, owner, repo) + skill_md, members = select_skill_paths(tree, skill_id) skill_dir = skill_md[: -len("/SKILL.md")] if "/" in skill_md else "" prefix = (skill_dir + "/") if skill_dir else "" files: dict[str, str] = {} for p in members: rel = p[len(prefix):] if prefix else p - raw = await client.get(f"{_GH_RAW}/{owner}/{repo}/{branch}/{p}") + raw = await client.get(f"{P_GH_RAW}/{owner}/{repo}/{branch}/{p}") if raw.status_code == 200: files[rel] = raw.text if "SKILL.md" not in files: raise ValueError("SKILL.md could not be fetched") - meta, _body = _parse_frontmatter(files["SKILL.md"]) + meta, _body = p_parse_frontmatter(files["SKILL.md"]) # Reuse the .swarm importer's content scan: flag files holding secret-shaped # literals (the author's leaked key, or a sketchy skill) so the user sees it # before installing from an unvetted repo. @@ -412,16 +412,16 @@ async def resolve_community_skill(source: str, skill_id: str) -> dict: "repo_url": f"https://github.com/{owner}/{repo}/tree/{branch}/{skill_dir}".rstrip("/"), "skill_id": skill_id, "files": files, - "scripts": sorted(rel for rel in files if _is_script_path(rel)), + "scripts": sorted(rel for rel in files if is_script_path(rel)), "secret_findings": secret_findings, } -async def _community_search(q: str, limit: int) -> dict: +async def p_community_search(q: str, limit: int) -> dict: """Live-proxy a query to the skills.sh wild registry. Not cached: it's a 600k-entry remote index, so we search it on demand rather than mirror it.""" async with httpx.AsyncClient(timeout=15.0, headers={"User-Agent": "openswarm"}) as client: - r = await client.get(_COMMUNITY_SEARCH_URL, params={"q": q or "skill"}) + r = await client.get(P_COMMUNITY_SEARCH_URL, params={"q": q or "skill"}) r.raise_for_status() data = r.json() skills = [] @@ -445,14 +445,14 @@ async def _community_search(q: str, limit: int) -> dict: return {"skills": skills, "total": len(skills), "offset": 0, "limit": limit, "source": "community"} -class _InstallRequest(BaseModel): +class p_InstallRequest(BaseModel): source: str skill_id: str confirm: bool = False @skill_registry.router.post("/install") -async def registry_install(req: _InstallRequest): +async def registry_install(req: p_InstallRequest): """Install a community (skills.sh) skill, in two honest steps. confirm=false (default): resolve + return a disclosure (the SKILL.md and the diff --git a/backend/apps/skills/skills.py b/backend/apps/skills/skills.py index 87fa659b..c76d3f9f 100644 --- a/backend/apps/skills/skills.py +++ b/backend/apps/skills/skills.py @@ -18,7 +18,7 @@ INDEX_PATH = os.path.join(SKILLS_DIR, ".skills_index.json") from backend.config.paths import SKILLS_WORKSPACE_DIR -def _load_index() -> dict[str, dict]: +def p_load_index() -> dict[str, dict]: """Read the skill index, never raising on a corrupt file. A truncated/garbled index (e.g. a crash mid-write before atomic writes existed) is moved aside so it's recoverable, and we start empty rather than bricking every skill op, @@ -44,13 +44,13 @@ def _load_index() -> dict[str, dict]: # writer. Today every index write runs on the single backend event-loop thread # (no await between a load and its save, so no lost-update race), but this stays # correct if a save ever moves to a thread pool the way settings' did. -_index_write_lock = threading.Lock() +p_index_write_lock = threading.Lock() -def _save_index(index: dict[str, dict]): +def p_save_index(index: dict[str, dict]): """Atomic index write: tmp file + os.replace so a crash mid-write can't leave a truncated index. Mirrors the settings store's write discipline.""" - with _index_write_lock: + with p_index_write_lock: os.makedirs(SKILLS_DIR, exist_ok=True) fd, tmp = tempfile.mkstemp(prefix=".skills_index.", suffix=".tmp", dir=SKILLS_DIR) try: @@ -78,7 +78,7 @@ def _save_index(index: dict[str, dict]): # `built_in: true` in the index. Users can edit the content (their # changes flow through to the matching agent's prompt on the next turn), # but they can't delete the file; the DELETE endpoint refuses with 409. -def _built_in_skill_registry() -> list[dict]: +def p_built_in_skill_registry() -> list[dict]: # Imported lazily so this module stays cheap to import from # everywhere (the skills outputs module pulls in pydantic+fastapi # transitively and we don't want a cycle). @@ -115,15 +115,15 @@ def _built_in_skill_registry() -> list[dict]: ] -def _seed_built_in_skills() -> None: +def p_seed_built_in_skills() -> None: """Copy each built-in skill into SKILLS_DIR if not already present, and ensure the index has the `built_in: true` flag so the UI and DELETE endpoint know to treat it specially. Idempotent; safe to call on every boot. Doesn't overwrite the file once it exists (so user edits are preserved across restarts and upgrades).""" - index = _load_index() + index = p_load_index() dirty = False - for entry in _built_in_skill_registry(): + for entry in p_built_in_skill_registry(): skill_id = entry["id"] fpath = os.path.join(SKILLS_DIR, f"{skill_id}.md") if not os.path.exists(fpath): @@ -149,7 +149,7 @@ def _seed_built_in_skills() -> None: index[skill_id] = meta dirty = True if dirty: - _save_index(index) + p_save_index(index) @asynccontextmanager @@ -157,7 +157,7 @@ async def skills_lifespan(): os.makedirs(SKILLS_DIR, exist_ok=True) os.makedirs(SKILLS_WORKSPACE_DIR, exist_ok=True) try: - _seed_built_in_skills() + p_seed_built_in_skills() except Exception: # Don't block app startup on a skill-seed failure; the worst # case is the user has to manually paste the skill in once. @@ -168,7 +168,7 @@ async def skills_lifespan(): skills = SubApp("skills", skills_lifespan) -def _skill_md_path(skill_id: str) -> tuple[str | None, str]: +def p_skill_md_path(skill_id: str) -> tuple[str | None, str]: """Resolve where a skill's markdown lives: (path, kind). A skill is either a folder (~/.claude/skills//SKILL.md, multi-file) or a @@ -183,7 +183,7 @@ def _skill_md_path(skill_id: str) -> tuple[str | None, str]: return None, "flat" -def _has_supporting_files(skill_dir: str) -> bool: +def p_has_supporting_files(skill_dir: str) -> bool: """True if a skill folder ships anything beyond its SKILL.md (scripts, templates).""" try: return any(e != "SKILL.md" and not e.startswith(".") for e in os.listdir(skill_dir)) @@ -191,12 +191,12 @@ def _has_supporting_files(skill_dir: str) -> bool: return False -def _build_skill(skill_id: str, content: str, md_path: str, kind: str, index: dict) -> Skill: +def p_build_skill(skill_id: str, content: str, md_path: str, kind: str, index: dict) -> Skill: """Assemble a Skill from disk + index, falling back to SKILL.md frontmatter for a folder skill the index hasn't catalogued (e.g. hand-dropped).""" meta = dict(index.get(skill_id, {})) if kind == "folder" and ("name" not in meta or "description" not in meta): - fm = _parse_skill_frontmatter(content) + fm = p_parse_skill_frontmatter(content) meta.setdefault("name", fm.get("name", "")) meta.setdefault("description", fm.get("description", "")) pretty = skill_id.replace("-", " ").replace("_", " ").title() @@ -210,14 +210,14 @@ def _build_skill(skill_id: str, content: str, md_path: str, kind: str, index: di command=meta.get("command", skill_id), built_in=bool(meta.get("built_in", False)), dir_path=skill_dir if kind == "folder" else "", - has_supporting_files=(kind == "folder" and _has_supporting_files(skill_dir)), + has_supporting_files=(kind == "folder" and p_has_supporting_files(skill_dir)), ) -def _sync_skills() -> list[Skill]: +def sync_skills() -> list[Skill]: """Sync skills from the filesystem, updating the index. Reads both layouts: legacy flat .md files and multi-file /SKILL.md folders.""" - index = _load_index() + index = p_load_index() result = [] seen: set[str] = set() @@ -234,23 +234,23 @@ def _sync_skills() -> list[Skill]: continue if skill_id in seen: continue - md_path, kind = _skill_md_path(skill_id) + md_path, kind = p_skill_md_path(skill_id) if not md_path: continue with open(md_path, encoding="utf-8") as f: content = f.read() seen.add(skill_id) - result.append(_build_skill(skill_id, content, md_path, kind, index)) + result.append(p_build_skill(skill_id, content, md_path, kind, index)) return result @skills.router.get("/list") async def list_skills(): - return {"skills": [s.model_dump() for s in _sync_skills()]} + return {"skills": [s.model_dump() for s in sync_skills()]} -def _parse_skill_frontmatter(raw: str) -> dict: +def p_parse_skill_frontmatter(raw: str) -> dict: """Extract YAML frontmatter fields from a SKILL.md file.""" if not raw.startswith("---"): return {} @@ -302,7 +302,7 @@ async def read_skill_workspace(workspace_id: str): except json.JSONDecodeError: pass - frontmatter = _parse_skill_frontmatter(skill_content) if skill_content else {} + frontmatter = p_parse_skill_frontmatter(skill_content) if skill_content else {} return { "skill_content": skill_content, @@ -313,20 +313,20 @@ async def read_skill_workspace(workspace_id: str): @skills.router.get("/{skill_id}") async def get_skill(skill_id: str): - for s in _sync_skills(): + for s in sync_skills(): if s.id == skill_id: return s.model_dump() raise HTTPException(status_code=404, detail="Skill not found") -def _safe_slug(raw: str) -> str: +def p_safe_slug(raw: str) -> str: slug = re.sub(r"[^a-zA-Z0-9_-]+", "-", (raw or "").strip().lower()).strip("-") return slug or "skill" -def _skill_exists(slug: str) -> bool: +def p_skill_exists(slug: str) -> bool: return ( - slug in _load_index() + slug in p_load_index() or os.path.isfile(os.path.join(SKILLS_DIR, f"{slug}.md")) or os.path.isdir(os.path.join(SKILLS_DIR, slug)) ) @@ -336,11 +336,11 @@ def unique_skill_slug(base: str) -> str: """A free slug for `base`, suffixing -2, -3, ... on collision. Lets a registry install land beside a same-named skill instead of silently overwriting the user's existing one.""" - slug = _safe_slug(base) - if not _skill_exists(slug): + slug = p_safe_slug(base) + if not p_skill_exists(slug): return slug i = 2 - while _skill_exists(f"{slug}-{i}"): + while p_skill_exists(f"{slug}-{i}"): i += 1 return f"{slug}-{i}" @@ -351,11 +351,11 @@ def write_folder_skill(skill_id: str, files: dict[str, str], meta: dict) -> Skil zip/.swarm import. Relpaths that try to escape the skill folder (../, abs paths) are dropped, an untrusted registry archive can't write outside its own dir.""" - slug = _safe_slug(skill_id) + slug = p_safe_slug(skill_id) base = os.path.join(SKILLS_DIR, slug) base_abs = os.path.abspath(base) # A folder write supersedes any legacy flat .md, so we never leave a - # phantom flat file shadowed by the folder (folder wins in _skill_md_path). + # phantom flat file shadowed by the folder (folder wins in p_skill_md_path). legacy_flat = os.path.join(SKILLS_DIR, f"{slug}.md") if os.path.isfile(legacy_flat): try: @@ -372,20 +372,20 @@ def write_folder_skill(skill_id: str, files: dict[str, str], meta: dict) -> Skil with open(dest, "w", encoding="utf-8") as f: f.write(content) - index = _load_index() + index = p_load_index() index[slug] = { "name": meta.get("name") or slug, "description": meta.get("description", ""), "command": meta.get("command", slug), } - _save_index(index) + p_save_index(index) - md_path, kind = _skill_md_path(slug) + md_path, kind = p_skill_md_path(slug) if not md_path: raise HTTPException(status_code=400, detail="skill had no SKILL.md") with open(md_path, encoding="utf-8") as f: content = f.read() - return _build_skill(slug, content, md_path, kind, index) + return p_build_skill(slug, content, md_path, kind, index) @skills.router.post("/create") @@ -402,7 +402,7 @@ async def create_skill(body: SkillCreate): @skills.router.put("/{skill_id}") async def update_skill(skill_id: str, body: SkillUpdate): - md_path, kind = _skill_md_path(skill_id) + md_path, kind = p_skill_md_path(skill_id) if not md_path: raise HTTPException(status_code=404, detail="Skill not found") @@ -410,7 +410,7 @@ async def update_skill(skill_id: str, body: SkillUpdate): with open(md_path, "w", encoding="utf-8") as f: f.write(body.content) - index = _load_index() + index = p_load_index() meta = index.get(skill_id, {}) if body.name is not None: meta["name"] = body.name @@ -419,18 +419,18 @@ async def update_skill(skill_id: str, body: SkillUpdate): if body.command is not None: meta["command"] = body.command index[skill_id] = meta - _save_index(index) + p_save_index(index) with open(md_path, encoding="utf-8") as f: content = f.read() - skill = _build_skill(skill_id, content, md_path, kind, index) + skill = p_build_skill(skill_id, content, md_path, kind, index) return {"ok": True, "skill": skill.model_dump()} @skills.router.delete("/{skill_id}") async def delete_skill(skill_id: str): - index = _load_index() + index = p_load_index() if index.get(skill_id, {}).get("built_in"): raise HTTPException( status_code=409, @@ -449,5 +449,5 @@ async def delete_skill(skill_id: str): if os.path.isfile(flat): os.remove(flat) index.pop(skill_id, None) - _save_index(index) + p_save_index(index) return {"ok": True} diff --git a/backend/tests/test_skill_registry_community.py b/backend/tests/test_skill_registry_community.py index cfa4b5ff..fe464e79 100644 --- a/backend/tests/test_skill_registry_community.py +++ b/backend/tests/test_skill_registry_community.py @@ -13,7 +13,7 @@ import os import pytest import backend.apps.skills.skills as skills_mod -from backend.apps.skill_registry.skill_registry import _select_skill_paths, _is_script_path +from backend.apps.skill_registry.skill_registry import select_skill_paths, is_script_path def test_selects_shortest_matching_skill_md_at_any_depth(): @@ -24,7 +24,7 @@ def test_selects_shortest_matching_skill_md_at_any_depth(): {"type": "blob", "path": "plugins/x/skills/pdftk/templates/form.txt"}, {"type": "blob", "path": "plugins/x/skills/other/SKILL.md"}, ] - skill_md, members = _select_skill_paths(tree, "pdftk") + skill_md, members = select_skill_paths(tree, "pdftk") assert skill_md == "plugins/x/skills/pdftk/SKILL.md" assert set(members) == { "plugins/x/skills/pdftk/SKILL.md", @@ -37,14 +37,14 @@ def test_selects_shortest_matching_skill_md_at_any_depth(): def test_top_level_skill_md(): tree = [{"type": "blob", "path": "pdftk/SKILL.md"}, {"type": "blob", "path": "pdftk/x.py"}] - skill_md, members = _select_skill_paths(tree, "pdftk") + skill_md, members = select_skill_paths(tree, "pdftk") assert skill_md == "pdftk/SKILL.md" assert "pdftk/x.py" in members def test_missing_skill_raises(): with pytest.raises(ValueError): - _select_skill_paths([{"type": "blob", "path": "a/SKILL.md"}], "nonexistent") + select_skill_paths([{"type": "blob", "path": "a/SKILL.md"}], "nonexistent") def test_ambiguous_match_picks_deterministically(): @@ -54,24 +54,24 @@ def test_ambiguous_match_picks_deterministically(): {"type": "blob", "path": "skills/pdf/SKILL.md"}, {"type": "blob", "path": "pdf/SKILL.md"}, ] - skill_md, _ = _select_skill_paths(tree, "pdf") + skill_md, _ = select_skill_paths(tree, "pdf") assert skill_md == "pdf/SKILL.md" # Without a top-level one, prefer skills//. tree2 = [ {"type": "blob", "path": "plugins/z/pdf/SKILL.md"}, {"type": "blob", "path": "skills/pdf/SKILL.md"}, ] - skill_md2, _ = _select_skill_paths(tree2, "pdf") + skill_md2, _ = select_skill_paths(tree2, "pdf") assert skill_md2 == "skills/pdf/SKILL.md" def test_github_headers_adds_token_when_set(monkeypatch): - from backend.apps.skill_registry.skill_registry import _github_headers + from backend.apps.skill_registry.skill_registry import github_headers monkeypatch.delenv("OPENSWARM_GITHUB_TOKEN", raising=False) monkeypatch.delenv("GITHUB_TOKEN", raising=False) - assert "Authorization" not in _github_headers() + assert "Authorization" not in github_headers() monkeypatch.setenv("OPENSWARM_GITHUB_TOKEN", "ghp_test") - assert _github_headers()["Authorization"] == "Bearer ghp_test" + assert github_headers()["Authorization"] == "Bearer ghp_test" def test_install_disclosure_flags_secret_shaped_files(): @@ -88,13 +88,13 @@ def test_install_disclosure_flags_secret_shaped_files(): def test_script_classification(): - assert _is_script_path("run.sh") - assert _is_script_path("helper.py") - assert _is_script_path("scripts/build.txt") # under a scripts/ dir - assert _is_script_path("bin/tool") - assert not _is_script_path("SKILL.md") - assert not _is_script_path("templates/form.html") - assert not _is_script_path("data.json") + assert is_script_path("run.sh") + assert is_script_path("helper.py") + assert is_script_path("scripts/build.txt") # under a scripts/ dir + assert is_script_path("bin/tool") + assert not is_script_path("SKILL.md") + assert not is_script_path("templates/form.html") + assert not is_script_path("data.json") # --------------------------------------------------------------------------- @@ -121,7 +121,7 @@ def test_write_folder_skill_lands_files_and_indexes(skills_dir): assert os.path.isfile(skills_dir / "pdf-tk" / "SKILL.md") assert os.path.isfile(skills_dir / "pdf-tk" / "scripts" / "run.sh") # Re-syncs and shows up in the list. - assert "pdf-tk" in {s.id for s in skills_mod._sync_skills()} + assert "pdf-tk" in {s.id for s in skills_mod.sync_skills()} def test_install_dedups_instead_of_clobbering_existing_skill(skills_dir): @@ -135,7 +135,7 @@ def test_install_dedups_instead_of_clobbering_existing_skill(skills_dir): assert f.read() == "MINE", "registry install clobbered the user's existing skill" with open(skills_dir / "pdf-2" / "SKILL.md", encoding="utf-8") as f: assert f.read() == "THEIRS" - ids = {s.id for s in skills_mod._sync_skills()} + ids = {s.id for s in skills_mod.sync_skills()} assert {"pdf", "pdf-2"} <= ids