fix: retain ClawHub owner through version and bundle requests
This commit is contained in:
90
tests/tools/test_clawhub_owner_http.py
Normal file
90
tests/tools/test_clawhub_owner_http.py
Normal file
@@ -0,0 +1,90 @@
|
||||
"""Owner-qualified ClawHub fetches retain identity across actual HTTP requests."""
|
||||
|
||||
import io
|
||||
import json
|
||||
import threading
|
||||
import zipfile
|
||||
from contextlib import contextmanager
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
from urllib.parse import parse_qs, urlsplit
|
||||
|
||||
from tools.skills_hub_clawhub import ClawHubSource
|
||||
|
||||
|
||||
@contextmanager
|
||||
def registry(*, fallback=False, mismatch=False):
|
||||
requests = []
|
||||
|
||||
class Handler(BaseHTTPRequestHandler):
|
||||
def log_message(self, *args):
|
||||
pass
|
||||
|
||||
def do_GET(self):
|
||||
url = urlsplit(self.path)
|
||||
query = parse_qs(url.query)
|
||||
requests.append((url.path, query))
|
||||
status = 200
|
||||
if query.get("owner") != ["alice"]:
|
||||
status, payload = 409, {"code": "AMBIGUOUS_SKILL_SLUG"}
|
||||
elif url.path.endswith("/download"):
|
||||
if fallback:
|
||||
status, payload = 404, {}
|
||||
else:
|
||||
buf = io.BytesIO()
|
||||
with zipfile.ZipFile(buf, "w") as archive:
|
||||
archive.writestr("SKILL.md", "# Alice fixture")
|
||||
payload = buf.getvalue()
|
||||
elif url.path.endswith("/versions"):
|
||||
payload = [{"version": "1.0"}]
|
||||
elif url.path.endswith("/versions/1.0"):
|
||||
payload = {"files": {"SKILL.md": "# Alice fixture"}}
|
||||
else:
|
||||
# Explicit owner must survive even when metadata omits it.
|
||||
payload = {"skill": {"slug": "collision"}}
|
||||
if mismatch:
|
||||
payload["owner"] = {"handle": "bob"}
|
||||
body = payload if isinstance(payload, bytes) else json.dumps(payload).encode()
|
||||
self.send_response(status)
|
||||
self.send_header("Content-Length", str(len(body)))
|
||||
self.end_headers()
|
||||
self.wfile.write(body)
|
||||
|
||||
server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
|
||||
thread = threading.Thread(target=server.serve_forever, daemon=True)
|
||||
thread.start()
|
||||
source = ClawHubSource()
|
||||
source.BASE_URL = f"http://127.0.0.1:{server.server_port}/api/v1"
|
||||
try:
|
||||
yield source, requests
|
||||
finally:
|
||||
server.shutdown()
|
||||
server.server_close()
|
||||
thread.join(timeout=5)
|
||||
|
||||
|
||||
def test_qualified_owner_survives_metadata_versions_and_download():
|
||||
for fallback in (False, True):
|
||||
with registry(fallback=fallback) as (source, requests):
|
||||
for identifier in ("@alice/collision", "clawhub/@alice/collision", "alice/skills/collision"):
|
||||
meta = source.inspect(identifier)
|
||||
assert meta is not None
|
||||
assert meta.identifier == "@alice/collision"
|
||||
bundle = source.fetch(identifier)
|
||||
assert bundle is not None
|
||||
assert bundle.identifier == "@alice/collision"
|
||||
assert bundle.files == {"SKILL.md": "# Alice fixture"}
|
||||
assert all(query.get("owner") == ["alice"] for _, query in requests)
|
||||
assert any(path.endswith("/versions") for path, _ in requests)
|
||||
assert any(path.endswith("/download") for path, _ in requests)
|
||||
if fallback:
|
||||
assert any(path.endswith("/versions/1.0") for path, _ in requests)
|
||||
|
||||
|
||||
def test_ambiguous_or_mismatched_owner_never_downloads():
|
||||
with registry() as (source, requests):
|
||||
assert source.fetch("collision") is None
|
||||
assert source.fetch("github-owner/repository/collision") is None
|
||||
assert len(requests) == 1
|
||||
with registry(mismatch=True) as (source, requests):
|
||||
assert source.fetch("@alice/collision") is None
|
||||
assert len(requests) == 1
|
||||
@@ -431,84 +431,6 @@ class TestClawHubSource(unittest.TestCase):
|
||||
self.assertIsNone(meta)
|
||||
mock_get.assert_called_once()
|
||||
|
||||
@patch("tools.skills_hub.httpx.get")
|
||||
def test_inspect_passes_owner_param_to_disambiguate_slug(self, mock_get):
|
||||
"""An @owner/slug identifier must reach the detail API as ?owner=.
|
||||
|
||||
ClawHub answers a slug claimed by multiple owners with 409
|
||||
AMBIGUOUS_SKILL_SLUG on the bare detail GET (#104117); the disambiguated
|
||||
form only resolves when the owner hint is forwarded as a query param."""
|
||||
mock_get.return_value = _MockResponse(
|
||||
status_code=200,
|
||||
json_data={
|
||||
"slug": "ponytail",
|
||||
"displayName": "ponytail",
|
||||
"summary": "Lazy senior dev mode",
|
||||
"owner": {"handle": "dietrichgebert"},
|
||||
"latestVersion": {"version": "1.0.0"},
|
||||
},
|
||||
)
|
||||
|
||||
meta = self.src.inspect("clawhub/@dietrichgebert/ponytail")
|
||||
|
||||
self.assertIsNotNone(meta)
|
||||
self.assertEqual(meta.extra.get("owner"), "dietrichgebert")
|
||||
args, kwargs = mock_get.call_args
|
||||
self.assertTrue(args[0].endswith("/skills/ponytail"))
|
||||
self.assertEqual(kwargs["params"], {"owner": "dietrichgebert"})
|
||||
|
||||
@patch("tools.skills_hub.httpx.get")
|
||||
def test_inspect_bare_slug_omits_owner_param(self, mock_get):
|
||||
"""Bare slugs keep the plain detail GET — no owner to forward."""
|
||||
mock_get.return_value = _MockResponse(
|
||||
status_code=200,
|
||||
json_data={
|
||||
"slug": "caldav-calendar",
|
||||
"displayName": "CalDAV Calendar",
|
||||
"summary": "Calendar integration",
|
||||
"latestVersion": {"version": "1.0.0"},
|
||||
},
|
||||
)
|
||||
|
||||
meta = self.src.inspect("caldav-calendar")
|
||||
|
||||
self.assertIsNotNone(meta)
|
||||
_, kwargs = mock_get.call_args
|
||||
self.assertIsNone(kwargs.get("params"))
|
||||
|
||||
@patch("tools.skills_hub._ssrf_safe_http_get")
|
||||
@patch("tools.skills_hub.httpx.get")
|
||||
def test_fetch_qualified_slug_resolves_ambiguous_slug_end_to_end(self, mock_get, mock_safe_get):
|
||||
"""install clawhub/@owner/slug must succeed for a multi-owner slug:
|
||||
the detail GET carries ?owner=, so ClawHub never answers 409."""
|
||||
def side_effect(url, *args, **kwargs):
|
||||
if url.endswith("/skills/ponytail"):
|
||||
return _MockResponse(
|
||||
status_code=200,
|
||||
json_data={
|
||||
"slug": "ponytail",
|
||||
"owner": {"handle": "dietrichgebert"},
|
||||
"latestVersion": {"version": "1.0.0"},
|
||||
},
|
||||
)
|
||||
if url.endswith("/skills/ponytail/versions/1.0.0"):
|
||||
return _MockResponse(
|
||||
status_code=200,
|
||||
json_data={"files": {"SKILL.md": "# Skill"}},
|
||||
)
|
||||
return _MockResponse(status_code=404, json_data={})
|
||||
|
||||
mock_get.side_effect = side_effect
|
||||
mock_safe_get.return_value = _MockResponse(status_code=200, text="# Skill")
|
||||
|
||||
bundle = self.src.fetch("@dietrichgebert/ponytail")
|
||||
|
||||
self.assertIsNotNone(bundle)
|
||||
self.assertEqual(bundle.name, "ponytail")
|
||||
self.assertEqual(bundle.files["SKILL.md"], "# Skill")
|
||||
detail_args, detail_kwargs = mock_get.call_args_list[0]
|
||||
self.assertTrue(detail_args[0].endswith("/skills/ponytail"))
|
||||
self.assertEqual(detail_kwargs["params"], {"owner": "dietrichgebert"})
|
||||
|
||||
|
||||
class TestClawHubCatalogWalkBounded(unittest.TestCase):
|
||||
|
||||
@@ -235,17 +235,22 @@ class ClawHubSource(GuardedFetchMixin, SkillSource):
|
||||
if detail is None:
|
||||
return None
|
||||
slug, skill_data = detail
|
||||
latest_version = self._resolve_latest_version(slug, skill_data)
|
||||
# Keep the requested identity even when the detail payload omits its owner.
|
||||
owner = self._parse_identifier(identifier)[1] or self._owner_from_payload(skill_data)
|
||||
owner_params = {"owner": owner} if owner else None
|
||||
latest_version = self._resolve_latest_version(slug, skill_data, owner=owner)
|
||||
if not latest_version:
|
||||
logger.warning("ClawHub fetch failed for %s: could not resolve latest version", slug)
|
||||
return None
|
||||
|
||||
# Primary: ZIP bundle from /download. Fallback: version metadata with
|
||||
# inline/raw content (files may sit under version_data["version"]).
|
||||
files = self._download_zip(slug, latest_version)
|
||||
files = self._download_zip(slug, latest_version, owner=owner)
|
||||
if "SKILL.md" not in files:
|
||||
version_data = self._get_json(f"{self.BASE_URL}/skills/{slug}/versions/{latest_version}")
|
||||
if isinstance(version_data, dict):
|
||||
version_data = self._get_json(
|
||||
f"{self.BASE_URL}/skills/{slug}/versions/{latest_version}", params=owner_params,
|
||||
)
|
||||
if isinstance(version_data, dict) and self._owner_matches(owner, version_data):
|
||||
files = self._extract_files(version_data) or files
|
||||
nested = version_data.get("version", {})
|
||||
if "SKILL.md" not in files and isinstance(nested, dict):
|
||||
@@ -255,14 +260,20 @@ class ClawHubSource(GuardedFetchMixin, SkillSource):
|
||||
"ClawHub fetch for %s resolved version %s but could not retrieve file content", slug, latest_version,
|
||||
)
|
||||
return None
|
||||
return SkillBundle(name=slug, files=files, source="clawhub", identifier=slug, trust_level="community")
|
||||
return SkillBundle(name=slug, files=files, source="clawhub",
|
||||
identifier=f"@{owner}/{slug}" if owner else slug, trust_level="community")
|
||||
|
||||
def inspect(self, identifier: str) -> Optional[SkillMeta]:
|
||||
detail = self._skill_detail(identifier)
|
||||
if detail is None:
|
||||
return None
|
||||
slug, data = detail
|
||||
return self._item_to_meta({**data, "slug": data.get("slug") or slug})
|
||||
meta = self._item_to_meta({**data, "slug": data.get("slug") or slug})
|
||||
owner = self._parse_identifier(identifier)[1] or self._owner_from_payload(data)
|
||||
if meta is not None and owner:
|
||||
meta.identifier = f"@{owner}/{slug}"
|
||||
meta.extra["owner"] = owner
|
||||
return meta
|
||||
|
||||
def _search_catalog(self, query: str, limit: int = 10) -> List[SkillMeta]:
|
||||
cache_key = f"clawhub_search_catalog_v1_{hashlib.md5(f'{query}|{limit}'.encode()).hexdigest()}"
|
||||
@@ -325,7 +336,8 @@ class ClawHubSource(GuardedFetchMixin, SkillSource):
|
||||
_cache_metas(cache_key, results)
|
||||
return results
|
||||
|
||||
def _resolve_latest_version(self, slug: str, skill_data: Dict[str, Any]) -> Optional[str]:
|
||||
def _resolve_latest_version(self, slug: str, skill_data: Dict[str, Any],
|
||||
owner: Optional[str] = None) -> Optional[str]:
|
||||
latest, tags = skill_data.get("latestVersion"), skill_data.get("tags")
|
||||
version = _first_str(
|
||||
latest.get("version") if isinstance(latest, dict) else None,
|
||||
@@ -333,7 +345,8 @@ class ClawHubSource(GuardedFetchMixin, SkillSource):
|
||||
)
|
||||
if version:
|
||||
return version
|
||||
vd = self._get_json(f"{self.BASE_URL}/skills/{slug}/versions")
|
||||
vd = self._get_json(f"{self.BASE_URL}/skills/{slug}/versions",
|
||||
params={"owner": owner} if owner else None)
|
||||
return _first_str(vd[0].get("version")) if isinstance(vd, list) and vd and isinstance(vd[0], dict) else None
|
||||
|
||||
def _fetch_owner_handle(self, slug: str) -> Optional[str]:
|
||||
@@ -439,16 +452,19 @@ class ClawHubSource(GuardedFetchMixin, SkillSource):
|
||||
files[fname] = content
|
||||
return files
|
||||
|
||||
def _download_zip(self, slug: str, version: str) -> Dict[str, str]:
|
||||
def _download_zip(self, slug: str, version: str, owner: Optional[str] = None) -> Dict[str, str]:
|
||||
"""Download the skill ZIP from /download and extract its text files."""
|
||||
import io
|
||||
import zipfile
|
||||
|
||||
files: Dict[str, str] = {}
|
||||
params = {"slug": slug, "version": version}
|
||||
if owner:
|
||||
params["owner"] = owner
|
||||
max_retries = 3
|
||||
for attempt in range(max_retries):
|
||||
try:
|
||||
resp = httpx.get(f"{self.BASE_URL}/download", params={"slug": slug, "version": version},
|
||||
resp = httpx.get(f"{self.BASE_URL}/download", params=params,
|
||||
timeout=30, follow_redirects=True)
|
||||
if resp.status_code == 429:
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user