[Bug] SDK downloadPackage/pull_closure target a nonexistent /download endpoint — every package pull 404s (TS + Python)

.changeset/sdk-download-presigned-1011.md

@@ -0,0 +1,5 @@
+---
+"@chronoai/ornn-sdk": patch
+---
+
+Fix package download in both SDKs. `downloadPackage` / `download_package` (and the `pullClosure` / `pull_closure` that build on them) targeted a `GET /skills/:id/versions/:version/download` endpoint that does not exist server-side, so every package pull 404'd and both README quickstarts were broken. They now resolve the package via the skill-detail `presignedPackageUrl` (honoring an optional `version`), fetch the bytes directly from object storage, and verify them against the skill's `skillHash` (SRI) when present. (#1011)

sdk/python/README.md

@@ -45,7 +45,10 @@ with OrnnClient(
 
     # Pull
     pkg = ornn.download_package(skill.id, skill.latest_version)
-    # pkg is raw bytes — write to disk, pass to zipfile, etc.
+    # pkg is raw bytes — write to disk, pass to zipfile, etc. Resolves the
+    # version's presigned package URL from the skill detail and fetches it
+    # directly; verifies the bytes against `skill_hash` (SRI) when present.
+    # version is optional — omit it to pull the latest.
 
     # Publish
     new_skill = ornn.publish(pkg)
@@ -88,7 +91,7 @@ Error codes follow [`docs/CONVENTIONS.md` §1.4](../../docs/CONVENTIONS.md) (low
 | `search(...)` | `GET /skill-search` — returns `SkillSearchResult` |
 | `get(guid_or_name, version=None)` | `GET /skills/:id` — returns `SkillDetail` |
 | `list_versions(guid_or_name)` | `GET /skills/:id/versions` — returns `list[SkillVersionEntry]` |
-| `download_package(guid, version)` | `GET /skills/:id/versions/:v/download` — returns `bytes` |
+| `download_package(guid, version=None)` | `GET /skills/:id` → fetch `presigned_package_url` (SRI-verified) — returns `bytes` |
 | `publish(zip_bytes, skip_validation=False)` | `POST /skills` — returns `SkillDetail` |
 | `update(id, metadata=..., zip_bytes=..., skip_validation=False)` | `PUT /skills/:id` — returns `SkillDetail` |
 | `delete(id)` | `DELETE /skills/:id` |

sdk/python/src/ornn_sdk/client.py

@@ -14,6 +14,7 @@
 
 from __future__ import annotations
 
+import hashlib
 from collections.abc import Callable
 from typing import Any
 from urllib.parse import urlencode
@@ -75,6 +76,10 @@ def __init__(
         self._base_url = base_url.rstrip("/")
         self._static_token = token
         self._token_resolver = token_resolver
+        # Retained for the one-shot presigned-URL fetch in
+        # download_package, which bypasses self._http (object storage is
+        # outside /api/v1 and unauthenticated).
+        self._timeout = timeout
         self._http = httpx.Client(
             base_url=f"{self._base_url}/api/v1",
             transport=transport,
@@ -143,16 +148,59 @@ def list_versions(self, guid_or_name: str) -> list[SkillVersionEntry]:
         data = self.request("GET", f"/skills/{_quote(guid_or_name)}/versions")
         return [SkillVersionEntry.from_dict(v) for v in data.get("items") or []]
 
-    def download_package(self, guid: str, version: str) -> bytes:
-        """Download a skill package ZIP. Returns raw bytes."""
-        path = f"/skills/{_quote(guid)}/versions/{_quote(version)}/download"
-        res = self._raw_request("GET", path)
+    def download_package(self, guid: str, version: str | None = None) -> bytes:
+        """Download a skill package ZIP. Returns raw bytes.
+
+        There is no ``/api/v1`` download endpoint: the bytes live in
+        object storage behind a time-limited presigned URL. This resolves
+        that URL via the skill-detail read (:meth:`get`), then fetches the
+        absolute URL directly with a one-shot ``httpx.get`` — NOT through
+        :meth:`_raw_request`, which would wrongly prefix ``/api/v1`` and
+        attach the NyxID bearer to an object-storage request.
+
+        When the detail carries ``skill_hash`` (hex SHA-256), the
+        downloaded bytes are re-hashed and compared (SRI-style); a
+        mismatch raises :class:`OrnnError` with code ``integrity_mismatch``.
+
+        ``version`` is optional — defaults to the skill's latest version
+        (resolved server-side) when omitted.
+        """
+        detail = self.get(guid, version=version)
+        url = detail.presigned_package_url
+        if not url:
+            ref = f"{guid}@{version}" if version else guid
+            raise OrnnError(
+                status=404,
+                code="package_not_found",
+                message=f"Ornn: no downloadable package for skill {ref}",
+            )
+        # Bare httpx.get — the presigned URL is absolute object storage,
+        # outside the /api/v1 surface and not authenticated with the
+        # NyxID bearer.
+        res = httpx.get(url, timeout=self._timeout, follow_redirects=True)
         if res.status_code >= 400:
-            raise _build_error(res)
+            raise OrnnError(
+                status=res.status_code,
+                code="package_download_failed",
+                message=f"Ornn: package download failed with HTTP {res.status_code}",
+            )
         # httpx exposes res.content as bytes at runtime; the typeshed
         # stub annotates it as Any. Cast explicitly so strict mypy is
         # happy without disabling the rule.
-        return bytes(res.content)
+        data = bytes(res.content)
+        if detail.skill_hash:
+            actual = hashlib.sha256(data).hexdigest()
+            if actual != detail.skill_hash:
+                ref = f"{guid}@{version}" if version else guid
+                raise OrnnError(
+                    status=0,
+                    code="integrity_mismatch",
+                    message=(
+                        f"Ornn: package integrity check failed for {ref} "
+                        f"(expected {detail.skill_hash}, got {actual})"
+                    ),
+                )
+        return data
 
     def resolve_closure(self, guid_or_name: str, *, version: str | None = None) -> ClosureResult:
         """Resolve the full transitive dependency closure of a skill (#968).

sdk/python/src/ornn_sdk/types.py

@@ -70,14 +70,27 @@ class SkillDetail(SkillSummary):
     storage_key: str | None = None
     shared_with_users: list[str] = field(default_factory=list)
     shared_with_orgs: list[str] = field(default_factory=list)
+    # Absolute, time-limited object-storage URL for the version's package
+    # ZIP (resolved server-side per ?version=). `download_package` fetches
+    # it directly (not via /api/v1). Absent when unreadable / no package.
+    presigned_package_url: str | None = None
+    # Hex SHA-256 of the package bytes — `download_package` verifies the
+    # downloaded bytes against it (SRI) when present.
+    skill_hash: str | None = None
 
     @classmethod
     def from_dict(cls, raw: dict[str, Any]) -> SkillDetail:
         base = SkillSummary.from_dict(raw).__dict__
         base.pop("_extra")
         # `ownerId` was removed from the wire in #581 — tolerate it
         # showing up on old API responses (treat as unknown extra).
-        known_extra = {"storageKey", "sharedWithUsers", "sharedWithOrgs"}
+        known_extra = {
+            "storageKey",
+            "sharedWithUsers",
+            "sharedWithOrgs",
+            "presignedPackageUrl",
+            "skillHash",
+        }
         summary_known = {
             "id",
             "name",
@@ -100,6 +113,8 @@ def from_dict(cls, raw: dict[str, Any]) -> SkillDetail:
             storage_key=raw.get("storageKey"),
             shared_with_users=list(raw.get("sharedWithUsers") or []),
             shared_with_orgs=list(raw.get("sharedWithOrgs") or []),
+            presigned_package_url=raw.get("presignedPackageUrl"),
+            skill_hash=raw.get("skillHash"),
             _extra=extra,
         )
 

sdk/python/tests/test_client.py

@@ -253,30 +253,79 @@ def test_list_versions_unwraps_items(self) -> None:
         assert versions[0].is_latest is True
 
 
+def _detail_data(**overrides: object) -> dict[str, object]:
+    base: dict[str, object] = {
+        "id": "abc",
+        "name": "abc",
+        "description": "",
+        "isPrivate": False,
+        "createdBy": "u1",
+        "createdOn": "2026-01-01T00:00:00Z",
+    }
+    base.update(overrides)
+    return base
+
+
+PRESIGNED = "https://obj.example.com/bucket/abc-1.0.zip?sig=xyz"
+
+
 class TestDownload:
     @respx.mock
-    def test_download_returns_raw_bytes(self) -> None:
+    def test_download_resolves_presigned_url_and_fetches_bytes(self) -> None:
+        # download_package now reads the skill detail for the presigned
+        # URL (#1011), then fetches that absolute object-storage URL.
         zip_bytes = b"PK\x03\x04\x01\x02\x03"
-        respx.get(f"{BASE}/api/v1/skills/abc/versions/1.0/download").respond(
+        detail = respx.get(f"{BASE}/api/v1/skills/abc").respond(
             200,
-            content=zip_bytes,
-            headers={"Content-Type": "application/zip"},
+            json={
+                "data": _detail_data(presignedPackageUrl=PRESIGNED),
+                "error": None,
+            },
+        )
+        obj = respx.get(PRESIGNED).respond(
+            200, content=zip_bytes, headers={"Content-Type": "application/zip"}
         )
         with make_client() as ornn:
             result = ornn.download_package("abc", "1.0")
         assert result == zip_bytes
+        # Detail read threads ?version= ; the object fetch is the bare URL.
+        assert detail.calls.last.request.url.params["version"] == "1.0"
+        assert obj.called
 
     @respx.mock
-    def test_download_raises_on_error(self) -> None:
-        respx.get(f"{BASE}/api/v1/skills/abc/versions/9.9/download").respond(
+    def test_download_version_optional_omits_version_param(self) -> None:
+        detail = respx.get(f"{BASE}/api/v1/skills/abc").respond(
+            200,
+            json={"data": _detail_data(presignedPackageUrl=PRESIGNED), "error": None},
+        )
+        respx.get(PRESIGNED).respond(200, content=b"PK")
+        with make_client() as ornn:
+            ornn.download_package("abc")
+        assert "version" not in detail.calls.last.request.url.params
+
+    @respx.mock
+    def test_download_raises_package_not_found_without_presigned_url(self) -> None:
+        respx.get(f"{BASE}/api/v1/skills/abc").respond(
+            200, json={"data": _detail_data(), "error": None}
+        )
+        with make_client() as ornn:
+            with pytest.raises(OrnnError) as excinfo:
+                ornn.download_package("abc", "1.0")
+        assert excinfo.value.status == 404
+        assert excinfo.value.code == "package_not_found"
+
+    @respx.mock
+    def test_download_raises_when_skill_read_404s(self) -> None:
+        # The detail read is the first hop; its 404 (RFC 7807) propagates.
+        respx.get(f"{BASE}/api/v1/skills/abc").respond(
             404,
             json={
                 "type": "https://github.com/.../ERRORS.md#resource_not_found",
                 "title": "Resource not found",
                 "status": 404,
                 "code": "resource_not_found",
-                "detail": "no such version",
-                "instance": "/v1/skills/abc/versions/9.9/download",
+                "detail": "no such skill",
+                "instance": "/v1/skills/abc",
             },
         )
         with make_client() as ornn:
@@ -285,6 +334,53 @@ def test_download_raises_on_error(self) -> None:
         assert excinfo.value.status == 404
         assert excinfo.value.code == "resource_not_found"
 
+    @respx.mock
+    def test_download_raises_package_download_failed_on_non_2xx(self) -> None:
+        respx.get(f"{BASE}/api/v1/skills/abc").respond(
+            200,
+            json={"data": _detail_data(presignedPackageUrl=PRESIGNED), "error": None},
+        )
+        # Object storage rejects the (now-expired) presigned URL.
+        respx.get(PRESIGNED).respond(403, text="expired")
+        with make_client() as ornn:
+            with pytest.raises(OrnnError) as excinfo:
+                ornn.download_package("abc", "1.0")
+        assert excinfo.value.status == 403
+        assert excinfo.value.code == "package_download_failed"
+
+    @respx.mock
+    def test_download_verifies_skill_hash_and_returns_bytes(self) -> None:
+        import hashlib
+
+        zip_bytes = b"PK\x03\x04"
+        expected = hashlib.sha256(zip_bytes).hexdigest()
+        respx.get(f"{BASE}/api/v1/skills/abc").respond(
+            200,
+            json={
+                "data": _detail_data(presignedPackageUrl=PRESIGNED, skillHash=expected),
+                "error": None,
+            },
+        )
+        respx.get(PRESIGNED).respond(200, content=zip_bytes)
+        with make_client() as ornn:
+            result = ornn.download_package("abc", "1.0")
+        assert result == zip_bytes
+
+    @respx.mock
+    def test_download_raises_integrity_mismatch_on_bad_hash(self) -> None:
+        respx.get(f"{BASE}/api/v1/skills/abc").respond(
+            200,
+            json={
+                "data": _detail_data(presignedPackageUrl=PRESIGNED, skillHash="deadbeef"),
+                "error": None,
+            },
+        )
+        respx.get(PRESIGNED).respond(200, content=b"PK\x03\x04")
+        with make_client() as ornn:
+            with pytest.raises(OrnnError) as excinfo:
+                ornn.download_package("abc", "1.0")
+        assert excinfo.value.code == "integrity_mismatch"
+
 
 class TestClosure:
     @respx.mock
@@ -374,10 +470,28 @@ def test_pull_closure_downloads_in_topo_order(self) -> None:
                 "error": None,
             },
         )
-        dl_c = respx.get(f"{BASE}/api/v1/skills/g-c/versions/1.0/download").respond(
+        # Per-node skill-detail reads (#1011) resolve each node's
+        # presigned URL; the bytes then come from object storage.
+        url_c = "https://obj.example.com/g-c.zip"
+        url_b = "https://obj.example.com/g-b.zip"
+        detail_c = respx.get(f"{BASE}/api/v1/skills/g-c").respond(
+            200,
+            json={
+                "data": _detail_data(id="g-c", name="g-c", presignedPackageUrl=url_c),
+                "error": None,
+            },
+        )
+        detail_b = respx.get(f"{BASE}/api/v1/skills/g-b").respond(
+            200,
+            json={
+                "data": _detail_data(id="g-b", name="g-b", presignedPackageUrl=url_b),
+                "error": None,
+            },
+        )
+        dl_c = respx.get(url_c).respond(
             200, content=b"PKc", headers={"Content-Type": "application/zip"}
         )
-        dl_b = respx.get(f"{BASE}/api/v1/skills/g-b/versions/1.0/download").respond(
+        dl_b = respx.get(url_b).respond(
             200, content=b"PKb", headers={"Content-Type": "application/zip"}
         )
         with make_client() as ornn:
@@ -386,6 +500,7 @@ def test_pull_closure_downloads_in_topo_order(self) -> None:
         # Downloads follow the closure order — c (deps-first) before b.
         assert [node.name for node, _ in packages] == ["c", "b"]
         assert packages[0][1] == b"PKc"
+        assert detail_c.called and detail_b.called
         assert dl_c.called and dl_b.called
 
 

sdk/typescript/README.md

@@ -46,8 +46,11 @@ const { items } = await ornn.search({ q: "pdf", scope: "public" });
 const skill = await ornn.get(items[0]!.id);
 
 // Pull
-const pkg = await ornn.downloadPackage(skill.id, skill.latestVersion!);
-// pkg is an ArrayBuffer — write to disk, unzip, etc.
+const pkg = await ornn.downloadPackage(skill.id, skill.latestVersion);
+// pkg is an ArrayBuffer — write to disk, unzip, etc. Resolves the
+// version's presigned package URL from the skill detail and fetches it
+// directly; verifies the bytes against `skillHash` (SRI) when present.
+// version is optional — omit it to pull the latest.
 
 // Publish
 const newSkill = await ornn.publish(pkg); // or a Blob / Uint8Array
@@ -93,7 +96,7 @@ Error codes follow [`docs/CONVENTIONS.md` §1.4](../../docs/CONVENTIONS.md) (low
 | `search(params)` | `GET /skill-search` |
 | `get(guidOrName, version?)` | `GET /skills/:id` |
 | `listVersions(guidOrName)` | `GET /skills/:id/versions` |
-| `downloadPackage(guid, version)` | `GET /skills/:id/versions/:version/download` (returns `ArrayBuffer`) |
+| `downloadPackage(guid, version?)` | `GET /skills/:id` → fetch `presignedPackageUrl` (SRI-verified; returns `ArrayBuffer`) |
 | `publish(zip, options?)` | `POST /skills` (`application/zip` body) |
 | `update(id, { metadata? | zip? }, options?)` | `PUT /skills/:id` |
 | `delete(id)` | `DELETE /skills/:id` |