ewilliams28/turnstone
1
0
Fork 0
mirror of https://github.com/turnstonelabs/turnstone.git synced 2026-08-12 23:12:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: harden batch skill install — 7 review items + OpenAPI snapshot

Browse Source 
- Race condition: wrap create_prompt_template in try/except, append
  to skipped on conflict instead of crashing
- HTTP timeout: per-request timeout (10s+5s connect) instead of shared
  15s pool; parallelize SKILL.md and resource fetches with semaphore
  (5 concurrent)
- Branch detection: return branch_explicit from _parse_github_url(),
  eliminate duplicated regex matching and type: ignore comments
- Content-length: check len(resp.content) after fetch instead of
  unreliable content-length header; add size check in batch path
- Rate limits: _check_rate_limit() inspects x-ratelimit-remaining,
  raises actionable error on 403, warns when remaining < 10
- Root resources: fix _find_resource_files skipping root-level
  resources like scripts/foo.sh for root SKILL.md
- resource_count: pass accurate count in update and install responses
- Regenerate openapi-console.json with new resource endpoints
This commit is contained in:
Patrick Buckley
2026-03-17 00:40:41 -07:00
committed by Patrick Buckley
parent 8957b9ce0e
commit 4b44d88401
3 changed files with 378 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sdk/typescript/openapi-console.json
+276
View File
@@ -2470,6 +2470,195 @@
}
}
},
"/v1/api/admin/skills/{skill_id}/resources": {
"get": {
"summary": "List resource files for a skill",
"operationId": "v1_api_admin_skills_{skill_id}_resources_get",
"tags": [
"Admin"
],
"parameters": [
{
"name": "skill_id",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": "Success",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ListSkillResourcesResponse"
}
}
}
}
}
},
"post": {
"summary": "Upload a resource file to a skill",
"operationId": "v1_api_admin_skills_{skill_id}_resources_post",
"tags": [
"Admin"
],
"parameters": [
{
"name": "skill_id",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
}
],
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/CreateSkillResourceRequest"
}
}
}
},
"responses": {
"200": {
"description": "Success",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SkillResourceInfo"
}
}
}
},
"400": {
"description": "Error 400",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
},
"404": {
"description": "Error 404",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
},
"409": {
"description": "Error 409",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
}
}
}
},
"/v1/api/admin/skills/{skill_id}/resources/{path}": {
"get": {
"summary": "Get a single skill resource by path",
"operationId": "v1_api_admin_skills_{skill_id}_resources_{path}_get",
"tags": [
"Admin"
],
"parameters": [
{
"name": "skill_id",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
},
{
"name": "path",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": "Success",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SkillResourceInfo"
}
}
}
},
"404": {
"description": "Error 404",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
}
}
},
"delete": {
"summary": "Delete a skill resource by path",
"operationId": "v1_api_admin_skills_{skill_id}_resources_{path}_delete",
"tags": [
"Admin"
],
"parameters": [
{
"name": "skill_id",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
},
{
"name": "path",
"in": "path",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": "Success"
},
"404": {
"description": "Error 404",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
}
}
}
},
"/v1/api/admin/memories": {
"get": {
"summary": "List structured memories",
@@ -6680,6 +6869,11 @@
"title": "Scan Version",
"type": "string"
},
"resource_count": {
"default": 0,
"title": "Resource Count",
"type": "integer"
},
"created": {
"title": "Created",
"type": "string"
@@ -7153,6 +7347,88 @@
"title": "ListSkillVersionsResponse",
"type": "object"
},
"SkillResourceInfo": {
"properties": {
"resource_id": {
"title": "Resource Id",
"type": "string"
},
"skill_id": {
"title": "Skill Id",
"type": "string"
},
"path": {
"title": "Path",
"type": "string"
},
"content": {
"default": "",
"title": "Content",
"type": "string"
},
"content_type": {
"default": "text/plain",
"title": "Content Type",
"type": "string"
},
"size": {
"default": 0,
"title": "Size",
"type": "integer"
},
"created": {
"title": "Created",
"type": "string"
}
},
"required": [
"resource_id",
"skill_id",
"path",
"created"
],
"title": "SkillResourceInfo",
"type": "object"
},
"CreateSkillResourceRequest": {
"properties": {
"path": {
"title": "Path",
"type": "string"
},
"content": {
"title": "Content",
"type": "string"
},
"content_type": {
"default": "text/plain",
"title": "Content Type",
"type": "string"
}
},
"required": [
"path",
"content"
],
"title": "CreateSkillResourceRequest",
"type": "object"
},
"ListSkillResourcesResponse": {
"properties": {
"resources": {
"items": {
"$ref": "#/components/schemas/SkillResourceInfo"
},
"title": "Resources",
"type": "array"
}
},
"required": [
"resources"
],
"title": "ListSkillResourcesResponse",
"type": "object"
},
"SkillSummary": {
"properties": {
"name": {
turnstone/console/server.py
+27 -22
View File
@@ -2534,7 +2534,8 @@ async def admin_update_skill(request: Request) -> JSONResponse:
)
updated_skill = storage.get_prompt_template(skill_id)
return JSONResponse(_skill_to_response(updated_skill))
rc_map = storage.count_skill_resources_bulk([skill_id])
return JSONResponse(_skill_to_response(updated_skill, resource_count=rc_map.get(skill_id, 0)))
async def admin_delete_skill(request: Request) -> JSONResponse:
@@ -3187,26 +3188,30 @@ async def admin_skill_install(request: Request) -> JSONResponse:
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,
)
try:
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,
)
except Exception:
skipped.append({"name": parsed.name, "reason": "conflict"})
continue
# Store bundled resources
for res_path, res_content in package.resources.items():
@@ -3229,7 +3234,7 @@ async def admin_skill_install(request: Request) -> JSONResponse:
skill = storage.get_prompt_template(skill_id)
if skill:
installed.append(_skill_to_response(skill))
installed.append(_skill_to_response(skill, resource_count=len(package.resources)))
if len(packages) == 1 and not installed:
reason = skipped[0]["reason"] if skipped else "unknown"
turnstone/core/skill_sources.py
+75 -36
View File
@@ -6,6 +6,7 @@ and :func:`fetch_skill_from_github` for fetching SKILL.md from GitHub repos.
from __future__ import annotations
import asyncio
import logging
import os
import re
@@ -120,19 +121,20 @@ class SkillsShClient:
return url
def _parse_github_url(url: str) -> tuple[str, str, str, str]:
"""Parse a GitHub URL into (owner, repo, branch, path).
def _parse_github_url(url: str) -> tuple[str, str, str, str, bool]:
"""Parse a GitHub URL into (owner, repo, branch, path, branch_explicit).
Returns ("", "", "", "") if URL doesn't match.
Returns ("", "", "", "", False) if URL doesn't match.
"""
m = _GITHUB_URL_RE.match(url)
if not m:
return ("", "", "", "")
return ("", "", "", "", False)
return (
m.group("owner"),
m.group("repo"),
m.group("branch") or "main",
m.group("path") or "",
bool(m.group("branch")),
)
@@ -150,8 +152,6 @@ def _find_resource_files(
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
@@ -165,21 +165,45 @@ def _find_resource_files(
return resource_files[:_MAX_RESOURCE_FILES]
def _check_rate_limit(resp: httpx.Response) -> None:
"""Raise SkillSourceError with guidance if GitHub rate limit is hit."""
if resp.status_code == 403:
remaining = resp.headers.get("x-ratelimit-remaining", "")
if remaining == "0":
raise SkillSourceError(
"GitHub API rate limit exceeded. "
"Set TURNSTONE_GITHUB_TOKEN env var for higher limits (5000 req/hr)."
)
remaining = resp.headers.get("x-ratelimit-remaining", "")
if remaining and remaining.isdigit() and int(remaining) < 10:
logger.warning("GitHub API rate limit low: %s remaining", remaining)
_FETCH_CONCURRENCY = 5
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
"""Fetch content for a list of resource files (concurrent)."""
if not resource_files:
return {}
sem = asyncio.Semaphore(_FETCH_CONCURRENCY)
async def _fetch_one(rf: dict[str, str]) -> tuple[str, str] | None:
async with sem:
try:
resp = await client.get(f"{raw_base}/{rf['full_path']}")
if resp.status_code == 200:
return rf["path"], resp.text
except httpx.HTTPError:
pass
return None
results = await asyncio.gather(*[_fetch_one(rf) for rf in resource_files])
return {path: content for r in results if r is not None for path, content in [r]}
async def fetch_skill_from_github(url: str) -> SkillPackage:
@@ -193,7 +217,7 @@ async def fetch_skill_from_github(url: str) -> SkillPackage:
Uses ``TURNSTONE_GITHUB_TOKEN`` env var for authenticated requests
(60 → 5000 req/hr rate limit headroom).
"""
owner, repo, branch, path = _parse_github_url(url)
owner, repo, branch, path, branch_explicit = _parse_github_url(url)
if not owner:
raise SkillSourceError(f"Could not parse GitHub URL: {url}")
@@ -203,7 +227,6 @@ async def fetch_skill_from_github(url: str) -> SkillPackage:
headers["Authorization"] = f"Bearer {token}"
# When branch isn't specified in URL, try main then master
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}"
@@ -233,7 +256,10 @@ async def fetch_skill_from_github(url: str) -> SkillPackage:
skill_md_content = ""
skill_md_dir = ""
resolved_branch = branch
async with httpx.AsyncClient(follow_redirects=True, timeout=15.0, headers=headers) as client:
_timeout = httpx.Timeout(10.0, connect=5.0)
async with httpx.AsyncClient(
follow_redirects=True, timeout=_timeout, headers=headers
) as client:
# Try each branch × candidate combination
for try_branch in branches_to_try:
raw_base = f"https://raw.githubusercontent.com/{owner}/{repo}/{try_branch}"
@@ -241,10 +267,9 @@ async def fetch_skill_from_github(url: str) -> SkillPackage:
try:
resp = await client.get(f"{raw_base}/{candidate}")
if resp.status_code == 200:
content_len = int(resp.headers.get("content-length", "0"))
if content_len > _MAX_SKILL_MD_SIZE:
if len(resp.content) > _MAX_SKILL_MD_SIZE:
continue
skill_md_content = resp.text[:_MAX_SKILL_MD_SIZE]
skill_md_content = resp.text
# Directory containing the SKILL.md
parts = candidate.rsplit("/", 1)
skill_md_dir = parts[0] if len(parts) > 1 else ""
@@ -270,6 +295,7 @@ async def fetch_skill_from_github(url: str) -> SkillPackage:
f"{api_base}/git/trees/{resolved_branch}",
params={"recursive": "1"},
)
_check_rate_limit(tree_resp)
if tree_resp.status_code == 200 and len(tree_resp.content) < 2 * 1024 * 1024:
tree_data = tree_resp.json()
rf = _find_resource_files(tree_data.get("tree", []), skill_md_dir)
@@ -304,7 +330,7 @@ async def fetch_skills_from_github_repo(url: str) -> list[SkillPackage]:
Used when a repo-level URL has no root SKILL.md (monorepo pattern).
"""
owner, repo, branch, url_path = _parse_github_url(url)
owner, repo, branch, url_path, branch_explicit = _parse_github_url(url)
if not owner:
raise SkillSourceError(f"Could not parse GitHub URL: {url}")
url_path = url_path.rstrip("/")
@@ -314,12 +340,14 @@ async def fetch_skills_from_github_repo(url: str) -> list[SkillPackage]:
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:
_timeout = httpx.Timeout(10.0, connect=5.0)
async with httpx.AsyncClient(
follow_redirects=True, timeout=_timeout, headers=headers
) as client:
# Find the tree with all SKILL.md files
tree_data: dict[str, Any] = {}
resolved_branch = branch
@@ -329,6 +357,7 @@ async def fetch_skills_from_github_repo(url: str) -> list[SkillPackage]:
f"{api_base}/git/trees/{try_branch}",
params={"recursive": "1"},
)
_check_rate_limit(resp)
if resp.status_code == 200 and len(resp.content) < 2 * 1024 * 1024:
tree_data = resp.json()
resolved_branch = try_branch
@@ -360,21 +389,31 @@ async def fetch_skills_from_github_repo(url: str) -> list[SkillPackage]:
raw_base = f"https://raw.githubusercontent.com/{owner}/{repo}/{resolved_branch}"
# Fetch all SKILL.md files concurrently
sem = asyncio.Semaphore(_FETCH_CONCURRENCY)
async def _fetch_skill_md(p: str) -> tuple[str, str] | None:
async with sem:
try:
r = await client.get(f"{raw_base}/{p}")
if r.status_code == 200 and len(r.content) <= _MAX_SKILL_MD_SIZE:
return p, r.text
except httpx.HTTPError:
pass
return None
md_results = await asyncio.gather(*[_fetch_skill_md(p) for p in skill_md_paths])
packages: list[SkillPackage] = []
for skill_md_path in skill_md_paths:
for result in md_results:
if result is None:
continue
skill_md_path, content = result
# 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)
@@ -382,7 +421,7 @@ async def fetch_skills_from_github_repo(url: str) -> list[SkillPackage]:
logger.debug("Skipping invalid SKILL.md at %s", skill_md_path)
continue
# Collect resources for this skill
# Collect resources for this skill (concurrent via helper)
rf = _find_resource_files(tree_items, skill_md_dir)
resources = await _fetch_resource_contents(client, raw_base, rf)