From 8957b9ce0edf3b9aa62a2cc8994f96692c452a73 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 17 Mar 2026 00:29:57 -0700 Subject: [PATCH] feat: batch install skills from multi-skill GitHub repos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a GitHub repo URL has no root SKILL.md (monorepo pattern like anthropics/skills), automatically scan the repo tree for all SKILL.md files and install every discovered skill in one operation. - Add fetch_skills_from_github_repo() — scans recursive tree, parses each SKILL.md, collects per-skill resources via shared helpers - Extract _find_resource_files() and _fetch_resource_contents() to eliminate duplication between single and batch fetch paths - Extend admin_skill_install to fall back to batch scanning when single-skill fetch returns 404 - Each skill gets a specific source_url pointing to its subdirectory - Backward compatible: single-skill repos return same response shape - Frontend handles both shapes with contextual toast messages - Filter tree scan to URL path subtree when path is provided - Cap at 50 skills per repo scan Also addresses review feedback: - Fix path prefix check (scripts/ not scriptsX/) - Use count_skill_resources_bulk for single skill GET - Add content field to SkillResourceInfo schema - Fix OpenAPI spec paths ({path} not {path:path}) - Check r.ok on resource upload promises - Preserve / in URL-encoded paths (split/map/join pattern) - URL-encode path in Python SDK delete method - Fix test_install_not_found to mock batch fallback - Fix test_search_empty_results for required q param - Narrow except clause to ValueError in batch parser --- sdk/typescript/src/console.ts | 2 +- sdk/typescript/src/types.ts | 1 + tests/test_skill_discovery_api.py | 19 ++- turnstone/api/console_schemas.py | 1 + turnstone/api/console_spec.py | 4 +- turnstone/console/server.py | 155 +++++++++++-------- turnstone/console/static/governance.js | 34 ++++- turnstone/core/skill_sources.py | 197 ++++++++++++++++++++----- turnstone/sdk/console.py | 5 +- 9 files changed, 305 insertions(+), 113 deletions(-) diff --git a/sdk/typescript/src/console.ts b/sdk/typescript/src/console.ts index 0cfa3c2f..e5054af0 100644 --- a/sdk/typescript/src/console.ts +++ b/sdk/typescript/src/console.ts @@ -316,7 +316,7 @@ export class TurnstoneConsole extends BaseClient { async deleteSkillResource(skillId: string, path: string): Promise { await this.request( "DELETE", - `/v1/api/admin/skills/${skillId}/resources/${encodeURIComponent(path)}`, + `/v1/api/admin/skills/${skillId}/resources/${path.split("/").map(encodeURIComponent).join("/")}`, ); } diff --git a/sdk/typescript/src/types.ts b/sdk/typescript/src/types.ts index 4aaee8c6..2fc92afb 100644 --- a/sdk/typescript/src/types.ts +++ b/sdk/typescript/src/types.ts @@ -247,6 +247,7 @@ export interface SkillResourceInfo { resource_id: string; skill_id: string; path: string; + content?: string; content_type: string; size: number; created: string; diff --git a/tests/test_skill_discovery_api.py b/tests/test_skill_discovery_api.py index 0338aa35..029b7780 100644 --- a/tests/test_skill_discovery_api.py +++ b/tests/test_skill_discovery_api.py @@ -175,11 +175,15 @@ class TestSkillDiscover: instance = mock_cls.return_value instance.search = AsyncMock(return_value=[]) - resp = client.get("/v1/api/admin/skills/discover") + resp = client.get("/v1/api/admin/skills/discover", params={"q": "test"}) assert resp.status_code == 200 assert resp.json()["skills"] == [] + def test_search_empty_query_rejected(self, client: TestClient) -> None: + resp = client.get("/v1/api/admin/skills/discover") + assert resp.status_code == 400 + def test_search_permission_denied(self, client_no_perm: TestClient) -> None: resp = client_no_perm.get("/v1/api/admin/skills/discover") assert resp.status_code == 403 @@ -345,10 +349,17 @@ class TestSkillInstall: assert resp.status_code == 409 def test_install_not_found(self, client: TestClient) -> None: - with patch( - "turnstone.core.skill_sources.fetch_skill_from_github", new_callable=AsyncMock - ) as mock_fetch: + with ( + patch( + "turnstone.core.skill_sources.fetch_skill_from_github", new_callable=AsyncMock + ) as mock_fetch, + patch( + "turnstone.core.skill_sources.fetch_skills_from_github_repo", + new_callable=AsyncMock, + ) as mock_batch, + ): mock_fetch.side_effect = SkillNotFoundError("SKILL.md not found") + mock_batch.side_effect = SkillNotFoundError("No SKILL.md files found") resp = client.post( "/v1/api/admin/skills/install", diff --git a/turnstone/api/console_schemas.py b/turnstone/api/console_schemas.py index f965ff75..78f47357 100644 --- a/turnstone/api/console_schemas.py +++ b/turnstone/api/console_schemas.py @@ -393,6 +393,7 @@ class SkillResourceInfo(BaseModel): resource_id: str skill_id: str path: str + content: str = "" content_type: str = "text/plain" size: int = 0 created: str diff --git a/turnstone/api/console_spec.py b/turnstone/api/console_spec.py index f7132efe..4a00689b 100644 --- a/turnstone/api/console_spec.py +++ b/turnstone/api/console_spec.py @@ -660,7 +660,7 @@ CONSOLE_ENDPOINTS: list[EndpointSpec] = [ tags=["Admin"], ), EndpointSpec( - "/v1/api/admin/skills/{skill_id}/resources/{path:path}", + "/v1/api/admin/skills/{skill_id}/resources/{path}", "GET", "Get a single skill resource by path", response_model=SkillResourceInfo, @@ -668,7 +668,7 @@ CONSOLE_ENDPOINTS: list[EndpointSpec] = [ tags=["Admin"], ), EndpointSpec( - "/v1/api/admin/skills/{skill_id}/resources/{path:path}", + "/v1/api/admin/skills/{skill_id}/resources/{path}", "DELETE", "Delete a skill resource by path", error_codes=[404], diff --git a/turnstone/console/server.py b/turnstone/console/server.py index 81c49035..4dcd0648 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -2333,8 +2333,8 @@ async def admin_get_skill(request: Request) -> JSONResponse: skill = storage.get_prompt_template(skill_id) if skill is None: return JSONResponse({"error": "Skill not found"}, status_code=404) - rc = len(storage.list_skill_resources(skill_id)) - return JSONResponse(_skill_to_response(skill, resource_count=rc)) + rc_map = storage.count_skill_resources_bulk([skill_id]) + return JSONResponse(_skill_to_response(skill, resource_count=rc_map.get(skill_id, 0))) async def admin_create_skill(request: Request) -> JSONResponse: @@ -2942,7 +2942,7 @@ async def admin_create_skill_resource(request: Request) -> JSONResponse: path = posixpath.normpath(path) if ".." in path.split("/") or "\x00" in path: return JSONResponse({"error": "Invalid path"}, status_code=400) - if not any(path.startswith(d.rstrip("/")) for d in _ALLOWED_RESOURCE_DIRS): + if not any(path.startswith(d) for d in _ALLOWED_RESOURCE_DIRS): return JSONResponse( {"error": "path must start with scripts/, references/, or assets/"}, status_code=400, @@ -3116,6 +3116,7 @@ async def admin_skill_install(request: Request) -> JSONResponse: SkillSourceError, SkillsShClient, fetch_skill_from_github, + fetch_skills_from_github_repo, ) from turnstone.core.web_helpers import read_json_or_400, require_storage_or_503 @@ -3143,12 +3144,16 @@ async def admin_skill_install(request: Request) -> JSONResponse: discovery_url = _get_discovery_url(request) client = SkillsShClient(base_url=discovery_url) github_url = await client.resolve_github_url(skill_id_param) - package = await fetch_skill_from_github(github_url) + packages = [await fetch_skill_from_github(github_url)] else: url = str(body.get("url", "")).strip() if not url: return JSONResponse({"error": "url is required"}, status_code=400) - package = await fetch_skill_from_github(url) + try: + packages = [await fetch_skill_from_github(url)] + except SkillNotFoundError: + # No root SKILL.md — try scanning for a multi-skill repo + packages = await fetch_skills_from_github_repo(url) except SkillNotFoundError as exc: return JSONResponse({"error": str(exc)}, status_code=404) except SkillSourceError as exc: @@ -3156,75 +3161,95 @@ async def admin_skill_install(request: Request) -> JSONResponse: except ValueError as exc: return JSONResponse({"error": str(exc)}, status_code=400) - # Check for duplicate by source_url - source_url = package.listing.source_url - if source_url: - existing = storage.get_skill_by_source_url(source_url) - if existing: - return JSONResponse( - {"error": f"Skill from '{source_url}' is already installed"}, - status_code=409, - ) - - # Check for duplicate by name - if storage.get_prompt_template_by_name(package.parsed.name): - return JSONResponse( - {"error": f"Skill name '{package.parsed.name}' already exists"}, - status_code=409, - ) - import json as _json audit_uid, ip = _audit_context(request) - skill_id = uuid.uuid4().hex - parsed = package.parsed - tags_str = _json.dumps(parsed.tags) - allowed_tools_str = _json.dumps(parsed.allowed_tools) - content = parsed.content[:32768] - token_estimate = len(content) // 4 if content else 0 + installed: list[dict[str, Any]] = [] + skipped: list[dict[str, str]] = [] - storage.create_prompt_template( - template_id=skill_id, - name=parsed.name, - category="general", - content=content, - variables="[]", - is_default=False, - org_id="", - created_by=audit_uid, - origin="source", - readonly=True, - description=parsed.description, - tags=tags_str, - source_url=source_url, - version=parsed.version, - author=parsed.author, - activation="named", - token_estimate=token_estimate, - allowed_tools=allowed_tools_str, - ) + for package in packages: + pkg_source_url = package.listing.source_url - # Store bundled resources - for path, content in package.resources.items(): - storage.create_skill_resource( - resource_id=uuid.uuid4().hex, - skill_id=skill_id, - path=path, + # Check for duplicate by source_url + if pkg_source_url and storage.get_skill_by_source_url(pkg_source_url): + skipped.append({"name": package.parsed.name, "reason": "already installed"}) + continue + + # Check for duplicate by name + if storage.get_prompt_template_by_name(package.parsed.name): + skipped.append({"name": package.parsed.name, "reason": "name exists"}) + continue + + skill_id = uuid.uuid4().hex + parsed = package.parsed + tags_str = _json.dumps(parsed.tags) + allowed_tools_str = _json.dumps(parsed.allowed_tools) + content = parsed.content[:32768] + token_estimate = len(content) // 4 if content else 0 + + storage.create_prompt_template( + template_id=skill_id, + name=parsed.name, + category="general", content=content, + variables="[]", + is_default=False, + org_id="", + created_by=audit_uid, + origin="source", + readonly=True, + description=parsed.description, + tags=tags_str, + source_url=pkg_source_url, + version=parsed.version, + author=parsed.author, + activation="named", + token_estimate=token_estimate, + allowed_tools=allowed_tools_str, ) - record_audit( - storage, - audit_uid, - "skill.install", - "skill", - skill_id, - {"name": parsed.name, "source": source, "source_url": source_url}, - ip, - ) + # Store bundled resources + for res_path, res_content in package.resources.items(): + storage.create_skill_resource( + resource_id=uuid.uuid4().hex, + skill_id=skill_id, + path=res_path, + content=res_content, + ) - skill = storage.get_prompt_template(skill_id) - return JSONResponse(_skill_to_response(skill)) + record_audit( + storage, + audit_uid, + "skill.install", + "skill", + skill_id, + {"name": parsed.name, "source": source, "source_url": pkg_source_url}, + ip, + ) + + skill = storage.get_prompt_template(skill_id) + if skill: + installed.append(_skill_to_response(skill)) + + if len(packages) == 1 and not installed: + reason = skipped[0]["reason"] if skipped else "unknown" + return JSONResponse( + {"error": f"Skill '{packages[0].parsed.name}' not installed: {reason}"}, + status_code=409, + ) + + # Single-skill install: return the skill object directly (backward compat) + if len(packages) == 1 and installed: + return JSONResponse(installed[0]) + + # Multi-skill batch: return summary + return JSONResponse( + { + "installed": installed, + "skipped": skipped, + "total": len(packages), + } + ) # --------------------------------------------------------------------------- diff --git a/turnstone/console/static/governance.js b/turnstone/console/static/governance.js index 32008e34..566c66bd 100644 --- a/turnstone/console/static/governance.js +++ b/turnstone/console/static/governance.js @@ -966,7 +966,10 @@ function submitCreateTemplate() { headers: { "Content-Type": "application/json" }, body: JSON.stringify(res), }, - ); + ).then(function (r) { + if (!r.ok) throw new Error("Upload failed for " + res.path); + return r.json(); + }); }); Promise.all(promises) .then(function () { @@ -1232,7 +1235,7 @@ function _loadSkillResources(skillId, readonly) { "/v1/api/admin/skills/" + skillId + "/resources/" + - encodeURIComponent(path), + path.split("/").map(encodeURIComponent).join("/"), { method: "DELETE" }, ) .then(function (r) { @@ -2291,12 +2294,29 @@ function submitGitHubImport() { }) .then(function (data) { hideGitHubImportModal(); - var tierMsg = data.scan_status ? " [" + data.scan_status + "]" : ""; - showToast("Skill installed: " + (data.name || "") + tierMsg); - // Refresh if we're on discover view - if (_skillCurrentView === "discover") { - searchSkillDiscover(); + if (data.installed) { + // Batch response from multi-skill repo + var count = data.installed.length; + var skipCount = (data.skipped || []).length; + var msg; + if (count === 0 && skipCount) { + msg = + "All " + + skipCount + + " skill" + + (skipCount !== 1 ? "s" : "") + + " already installed"; + } else { + msg = count + " skill" + (count !== 1 ? "s" : "") + " installed"; + if (skipCount) msg += " (" + skipCount + " already installed)"; + } + showToast(msg); + } else { + // Single skill response (backward compat) + var tierMsg = data.scan_status ? " [" + data.scan_status + "]" : ""; + showToast("Skill installed: " + (data.name || "") + tierMsg); } + loadGovSkills(); }) .catch(function (e) { errEl.textContent = e.message; diff --git a/turnstone/core/skill_sources.py b/turnstone/core/skill_sources.py index 68cf3ddd..a3ac3f3f 100644 --- a/turnstone/core/skill_sources.py +++ b/turnstone/core/skill_sources.py @@ -136,6 +136,52 @@ def _parse_github_url(url: str) -> tuple[str, str, str, str]: ) +def _find_resource_files( + tree_items: list[dict[str, Any]], skill_md_dir: str +) -> list[dict[str, str]]: + """Filter tree items to resource files relative to a SKILL.md directory.""" + resource_files: list[dict[str, str]] = [] + for item in tree_items: + if item.get("type") != "blob": + continue + item_path: str = item.get("path", "") + rel_path = item_path + if skill_md_dir: + if not item_path.startswith(f"{skill_md_dir}/"): + continue + rel_path = item_path[len(skill_md_dir) + 1 :] + elif "/" in item_path: + continue # root-level SKILL.md: only root-level resources + first_seg = rel_path.split("/")[0] if "/" in rel_path else "" + if first_seg not in _RESOURCE_DIRS: + continue + ext = os.path.splitext(rel_path)[1].lower() + if ext not in _TEXT_EXTENSIONS: + continue + size = item.get("size", 0) + if size > _MAX_RESOURCE_SIZE: + continue + resource_files.append({"path": rel_path, "full_path": item_path}) + return resource_files[:_MAX_RESOURCE_FILES] + + +async def _fetch_resource_contents( + client: httpx.AsyncClient, + raw_base: str, + resource_files: list[dict[str, str]], +) -> dict[str, str]: + """Fetch content for a list of resource files.""" + resources: dict[str, str] = {} + for rf in resource_files: + try: + resp = await client.get(f"{raw_base}/{rf['full_path']}") + if resp.status_code == 200: + resources[rf["path"]] = resp.text + except httpx.HTTPError: + continue + return resources + + async def fetch_skill_from_github(url: str) -> SkillPackage: """Fetch a SKILL.md and bundled resources from a GitHub repository. @@ -226,49 +272,134 @@ async def fetch_skill_from_github(url: str) -> SkillPackage: ) if tree_resp.status_code == 200 and len(tree_resp.content) < 2 * 1024 * 1024: tree_data = tree_resp.json() - resource_files: list[dict[str, Any]] = [] - for item in tree_data.get("tree", []): - if item.get("type") != "blob": - continue - item_path: str = item.get("path", "") - # Filter to resource dirs relative to SKILL.md location - rel_path = item_path - if skill_md_dir: - if not item_path.startswith(f"{skill_md_dir}/"): - continue - rel_path = item_path[len(skill_md_dir) + 1 :] - # Check if it's in a resource directory - first_seg = rel_path.split("/")[0] if "/" in rel_path else "" - if first_seg not in _RESOURCE_DIRS: - continue - # Filter to text-safe extensions only - ext = os.path.splitext(rel_path)[1].lower() - if ext not in _TEXT_EXTENSIONS: - continue - size = item.get("size", 0) - if size > _MAX_RESOURCE_SIZE: - continue - resource_files.append({"path": rel_path, "full_path": item_path}) - - # Fetch up to MAX files - for rf in resource_files[:_MAX_RESOURCE_FILES]: - try: - content_resp = await client.get(f"{raw_base}/{rf['full_path']}") - if content_resp.status_code == 200: - resources[rf["path"]] = content_resp.text - except httpx.HTTPError: - continue + rf = _find_resource_files(tree_data.get("tree", []), skill_md_dir) + resources = await _fetch_resource_contents(client, raw_base, rf) except httpx.HTTPError: logger.debug("Failed to fetch resource tree for %s/%s", owner, repo) + # Build a per-skill source URL pointing to the specific subdirectory + if skill_md_dir: + specific_url = f"https://github.com/{owner}/{repo}/tree/{resolved_branch}/{skill_md_dir}" + else: + specific_url = url + listing = SkillListing( id=f"{owner}/{repo}/{parsed.name}", name=parsed.name, description=parsed.description, author=parsed.author, source="github", - source_url=url, + source_url=specific_url, tags=parsed.tags, ) return SkillPackage(listing=listing, parsed=parsed, resources=resources) + + +_MAX_SKILLS_PER_REPO = 50 + + +async def fetch_skills_from_github_repo(url: str) -> list[SkillPackage]: + """Scan a GitHub repo for all SKILL.md files and return each as a package. + + Used when a repo-level URL has no root SKILL.md (monorepo pattern). + """ + owner, repo, branch, url_path = _parse_github_url(url) + if not owner: + raise SkillSourceError(f"Could not parse GitHub URL: {url}") + url_path = url_path.rstrip("/") + + headers: dict[str, str] = {"Accept": "application/vnd.github.v3+json"} + token = os.environ.get("TURNSTONE_GITHUB_TOKEN", "") + if token: + headers["Authorization"] = f"Bearer {token}" + + branch_explicit = bool(_GITHUB_URL_RE.match(url) and _GITHUB_URL_RE.match(url).group("branch")) # type: ignore[union-attr] + branches_to_try = [branch] if branch_explicit else ["main", "master"] + + api_base = f"https://api.github.com/repos/{owner}/{repo}" + + async with httpx.AsyncClient(follow_redirects=True, timeout=15.0, headers=headers) as client: + # Find the tree with all SKILL.md files + tree_data: dict[str, Any] = {} + resolved_branch = branch + for try_branch in branches_to_try: + try: + resp = await client.get( + f"{api_base}/git/trees/{try_branch}", + params={"recursive": "1"}, + ) + if resp.status_code == 200 and len(resp.content) < 2 * 1024 * 1024: + tree_data = resp.json() + resolved_branch = try_branch + break + except httpx.HTTPError: + continue + + if not tree_data: + raise SkillSourceError(f"Could not fetch repo tree for {owner}/{repo}") + + # Find all SKILL.md files in the tree (filtered to URL path if provided) + skill_md_paths: list[str] = [] + tree_items = tree_data.get("tree", []) + for item in tree_items: + if item.get("type") != "blob": + continue + p: str = item.get("path", "") + if not (p.endswith("/SKILL.md") or p == "SKILL.md"): + continue + if url_path and not p.startswith(f"{url_path}/") and p != url_path: + continue + skill_md_paths.append(p) + + if not skill_md_paths: + raise SkillNotFoundError(f"No SKILL.md files found in {owner}/{repo}") + + # Cap to prevent abuse + skill_md_paths = skill_md_paths[:_MAX_SKILLS_PER_REPO] + + raw_base = f"https://raw.githubusercontent.com/{owner}/{repo}/{resolved_branch}" + + packages: list[SkillPackage] = [] + for skill_md_path in skill_md_paths: + # Determine directory containing this SKILL.md + parts = skill_md_path.rsplit("/", 1) + skill_md_dir = parts[0] if len(parts) > 1 else "" + + # Fetch SKILL.md content + try: + resp = await client.get(f"{raw_base}/{skill_md_path}") + if resp.status_code != 200: + continue + content = resp.text[:_MAX_SKILL_MD_SIZE] + except httpx.HTTPError: + continue + + # Parse — skip if invalid + try: + parsed = parse_skill_md(content) + except ValueError: + logger.debug("Skipping invalid SKILL.md at %s", skill_md_path) + continue + + # Collect resources for this skill + rf = _find_resource_files(tree_items, skill_md_dir) + resources = await _fetch_resource_contents(client, raw_base, rf) + + specific_url = ( + f"https://github.com/{owner}/{repo}/tree/{resolved_branch}/{skill_md_dir}" + if skill_md_dir + else url + ) + listing = SkillListing( + id=f"{owner}/{repo}/{parsed.name}", + name=parsed.name, + description=parsed.description, + author=parsed.author, + source="github", + source_url=specific_url, + tags=parsed.tags, + ) + packages.append(SkillPackage(listing=listing, parsed=parsed, resources=resources)) + + return packages diff --git a/turnstone/sdk/console.py b/turnstone/sdk/console.py index 8a246199..8ecc1113 100644 --- a/turnstone/sdk/console.py +++ b/turnstone/sdk/console.py @@ -475,9 +475,12 @@ class AsyncTurnstoneConsole(_BaseClient): async def delete_skill_resource(self, skill_id: str, path: str) -> StatusResponse: """Delete a skill resource by path.""" + from urllib.parse import quote + + encoded = quote(path, safe="/") return await self._request( "DELETE", - f"/v1/api/admin/skills/{skill_id}/resources/{path}", + f"/v1/api/admin/skills/{skill_id}/resources/{encoded}", response_model=StatusResponse, )