diff --git a/CLAUDE.md b/CLAUDE.md index 12fc306..93c9965 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,8 +28,8 @@ make coverage # Run tests with coverage report poetry run pytest tests/test_auth.py -k "test_name" # Linting & formatting -make lint # Run all linters (pyright, flake8, codespell, format check) -make format # Auto-format code (autoflake, isort, black) +make lint # Run all linters (ruff check, ruff format --check, pyright) +make format # Auto-format code (ruff check --fix, ruff format) ``` ## Dependency Compatibility Matrix diff --git a/README.md b/README.md index 73a432d..027fb05 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,11 @@ - [Get Prompt](#get-prompt) - [Get Prompt Metadata](#get-prompt-metadata) - [Delete Prompt](#delete-prompt) + - [Skills](#skills) + - [Listing Skills](#listing-skills) + - [Listing Files in a Skill](#listing-files-in-a-skill) + - [Reading a File from a Skill](#reading-a-file-from-a-skill) + - [Downloading a Skill](#downloading-a-skill) - [Applications](#applications) - [List Applications](#list-applications) - [Get Application by Id](#get-application-by-id) @@ -795,6 +800,227 @@ client.prompts.delete("prompts/my-bucket/my-folder/my-prompt") await async_client.prompts.delete("prompts/my-bucket/my-folder/my-prompt") ``` +### Skills + +A DIAL *skill* is a folder-shaped resource served by DIAL Core's `/v2/skills` +API: a mandatory `SKILL.md` manifest plus an arbitrary hierarchy of bundled +files, addressed as a unit at `skills/{bucket}/{path}`. + +> [!NOTE] +> The `/v2/skills` endpoints are marked as preview in DIAL Core, so their +> contract may still change. The client currently supports the read +> operations; writes are tracked separately. + +Unlike the other resources, `client.skills` is not called with a URL you build +yourself. It is a *reference* that you narrow step by step, and each step +returns a new reference: + +```python +skill = client.skills / "writing" / "tone-of-voice" +# equivalently: client.skills(path="writing/tone-of-voice") +``` + +References are immutable, validate every path segment as they are built, and +issue no request until a terminal call (`list()`, `read()`, `download()`, +`stream()`, `stream_download()`). Building one is identical for the sync and +async clients — only the terminal call is awaited. + +A reference points at your own bucket unless told otherwise. Use `bucket=` for +a shared bucket such as `public`, and `url=` to follow an entry returned by a +listing: + +```python +client.skills(bucket="public") / "demo" / "azure-resource-visualizer" + +appdata = client.my_appdata() +client.skills(bucket=appdata.user_bucket, path=f"appdata/{appdata.app_name}") +``` + +#### Listing Skills + +`list()` returns the skills and grouping folders at the reference. With no +narrowing it lists your bucket root: + +```python +# Sync +listing = client.skills.list() +# Async +listing = await async_client.skills.list() + +for item in listing.items or []: + # "ITEM" is a skill, "FOLDER" is a grouping folder + print(item.node_type, item.url) + + # Follow either one with url= + nested = client.skills(url=item.url) +``` + +Narrow first to list a grouping folder, and pass the listing options to the +terminal call: + +```python +page = (client.skills / "writing").list(recursive=True, limit=1000) +``` + +Example of the response: + +```python +SkillMetadata( + name="writing", + parent_path=None, + bucket="my-bucket", + url="skills/my-bucket/writing/", + node_type="FOLDER", + resource_type="SKILL", + next_token=None, + items=[ + SkillItem( + name="tone-of-voice", + parent_path="writing", + bucket="my-bucket", + url="skills/my-bucket/writing/tone-of-voice", + node_type="ITEM", + resource_type="SKILL", + created_at=1724836229736, + updated_at=1724836248936, + author="user@example.com", + etag=None, + ) + ], +) +``` + +> [!NOTE] +> DIAL Core builds this listing without reading each skill's marker, so +> `etag` is always `None` here and the skill's `name`/`description` from +> `SKILL.md` are not included. Read `SKILL.md` itself if you need them. + +#### Listing Files in a Skill + +`skill.files` is a reference to the skill's bundled files. A page may hold +fewer entries than `limit`, so follow `next_token` until it is `None` — +building the reference once and varying only the token: + +```python +skill = client.skills / "writing" / "tone-of-voice" +files = skill.files + +token = None +while True: + page = files.list(recursive=True, limit=1000, token=token) + for item in page.items or []: + print(item.node_type, item.url) + token = page.next_token + if token is None: + break +``` + +Narrow to a subfolder the same way as anywhere else: + +```python +page = await (async_skill.files / "references").list() +``` + +Example of the response: + +```python +SkillFileMetadata( + name="files", + parent_path="writing/tone-of-voice", + bucket="my-bucket", + url="skills/my-bucket/writing/tone-of-voice/files/", + node_type="FOLDER", + resource_type="SKILL", + next_token=None, + items=[ + SkillFileItem( + name="SKILL.md", + parent_path="writing/tone-of-voice/files", + bucket="my-bucket", + url="skills/my-bucket/writing/tone-of-voice/files/SKILL.md", + node_type="ITEM", + resource_type="SKILL", + updated_at=1724836248936, + ), + SkillFileItem( + # A subfolder, as returned by a non-recursive listing. + name="references", + parent_path="writing/tone-of-voice/files", + bucket="my-bucket", + url="skills/my-bucket/writing/tone-of-voice/files/references/", + node_type="FOLDER", + resource_type="SKILL", + ), + ], +) +``` + +> [!NOTE] +> The two modes answer different questions. `recursive=True` flattens the +> tree: every file at every depth, no folder entries at all, with +> `parent_path` showing where each file sits. A non-recursive listing returns +> the immediate children, folders included, distinguished by +> `node_type == "FOLDER"`. Empty folders never appear in either mode. + +Unlike the `/v1` files listing, these entries are sparse: no +`content_length`, no `content_type`, and in observed responses no `etag` +either. Folder entries carry no `updated_at`. Treat every field except +`name`, `url`, `node_type` and `resource_type` as optional here. + +#### Reading a File from a Skill + +Name the file with `path=`, relative to the skill root, then `read()`: + +```python +# Sync +manifest = skill.files(path="SKILL.md").read() +print(manifest.get_content().decode()) + +# Async +manifest = await async_skill.files(path="SKILL.md").read() +schema = await async_skill.files(path="references/api-schema.md").read() +await schema.awrite_to("api-schema.md") +``` + +An entry from a files listing can be followed directly, without slicing its +url apart: + +```python +for item in files.list().items or []: + if item.node_type == "ITEM": + content = skill.files(url=item.url).read() +``` + +The async client can stream instead, which avoids holding the file in memory: + +```python +async with async_skill.files(path="assets/logo.png").stream() as file: + await file.awrite_to("logo.png") +``` + +#### Downloading a Skill + +`download()` fetches the whole skill as a ZIP archive: + +```python +# Sync +archive = skill.download() +archive.write_to("tone-of-voice.zip") + +# Async, streamed +async with async_skill.stream_download() as archive: + await archive.awrite_to("tone-of-voice.zip") +``` + +DIAL Core sends no `Content-Disposition` for this endpoint, so `filename` +defaults to the skill name with a `.zip` suffix (`tone-of-voice.zip` above). +The response's `ETag` header carries the skill's aggregate etag: + +```python +etag = archive.headers["etag"] +``` + + ### Applications #### List Applications diff --git a/aidial_client/_client.py b/aidial_client/_client.py index 03eb914..fd26676 100644 --- a/aidial_client/_client.py +++ b/aidial_client/_client.py @@ -15,7 +15,8 @@ validate_auth, ) from aidial_client._constants import ( - API_PREFIX, + API_PREFIX_V1, + API_PREFIX_V2, DEFAULT_MAX_RETRIES, DEFAULT_TIMEOUT, OPENAI_PREFIX, @@ -71,7 +72,11 @@ def is_dial_url(self, absolute_url: str) -> bool: @property def api_url(self) -> str: - return urljoin(self._base_url, API_PREFIX) + return urljoin(self._base_url, API_PREFIX_V1) + + @property + def api_url_v2(self) -> str: + return urljoin(self._base_url, API_PREFIX_V2) @property def base_url(self) -> str: @@ -110,6 +115,11 @@ def _init_resources(self) -> None: metadata=self.metadata, dial_api_url=self.api_url, ) + self.skills = resources.SkillsRef( + http_client=self._http_client, + dial_api_url=self.api_url_v2, + resolve_bucket=self.my_bucket, + ) self.deployments = resources.Deployments(http_client=self._http_client) self.application = resources.Application(http_client=self._http_client) self.toolset = resources.Toolset(http_client=self._http_client) @@ -211,6 +221,11 @@ def _init_resources(self) -> None: metadata=self.metadata, dial_api_url=self.api_url, ) + self.skills = resources.AsyncSkillsRef( + http_client=self._http_client, + dial_api_url=self.api_url_v2, + resolve_bucket=self.my_bucket, + ) self.deployments = resources.AsyncDeployments( http_client=self._http_client ) diff --git a/aidial_client/_constants.py b/aidial_client/_constants.py index bdfc9c6..58243e6 100644 --- a/aidial_client/_constants.py +++ b/aidial_client/_constants.py @@ -9,9 +9,13 @@ ) INITIAL_RETRY_DELAY = 0.5 MAX_RETRY_DELAY = 8.0 -API_PREFIX = "v1/" -METADATA_PREFIX = urljoin(API_PREFIX, "metadata/") -FILES_PREFIX = urljoin(API_PREFIX, "files/") +API_PREFIX_V1 = "v1/" +METADATA_PREFIX_V1 = urljoin(API_PREFIX_V1, "metadata/") +FILES_PREFIX = urljoin(API_PREFIX_V1, "files/") + +# DIAL Core exposes folder-shaped resources (agent skills) under /v2. +API_PREFIX_V2 = "v2/" +METADATA_PREFIX_V2 = urljoin(API_PREFIX_V2, "metadata/") OPENAI_PREFIX = "openai/" diff --git a/aidial_client/helpers/storage_resource.py b/aidial_client/helpers/storage_resource.py index d34075a..15fe4a6 100644 --- a/aidial_client/helpers/storage_resource.py +++ b/aidial_client/helpers/storage_resource.py @@ -2,14 +2,40 @@ from typing import Literal, cast, get_args from urllib.parse import quote, unquote, urljoin, urlparse, urlsplit +import httpx +from typing_extensions import assert_never + from aidial_client._compatibility.pydantic_v1 import BaseModel -from aidial_client._constants import API_PREFIX -from aidial_client._exception import InvalidDialURLError, NotDialURLError +from aidial_client._constants import API_PREFIX_V1, API_PREFIX_V2 +from aidial_client._exception import ( + DialException, + EtagMismatchError, + InvalidDialURLError, + NotDialURLError, + ResourceNotFoundError, +) from aidial_client._internal_types._http_request import FinalRequestOptions from aidial_client._utils._dict import remove_none from aidial_client.helpers._url import enforce_trailing_slash -StorageResourceType = Literal["files", "conversations", "prompts"] +StorageResourceTypeV1 = Literal["files", "conversations", "prompts"] +"""Resource types served by the /v1 storage API.""" + +StorageResourceTypeV2 = Literal["skills"] +"""Folder-shaped resource types served by the /v2 API.""" + +AnyStorageResourceType = StorageResourceTypeV1 | StorageResourceTypeV2 + + +def api_prefix_for(resource_type: AnyStorageResourceType) -> str: + """The API prefix that serves ``resource_type``.""" + match resource_type: + case "files" | "conversations" | "prompts": + return API_PREFIX_V1 + case "skills": + return API_PREFIX_V2 + case _: + assert_never(resource_type) def _percent_encode_relative_url(url: str) -> str: @@ -28,12 +54,84 @@ def _percent_encode_relative_url(url: str) -> str: return "/".join(quote(unquote(seg), safe="") for seg in segments) +def split_relative_segments( + path: str, + param: str, + *, + allow_trailing_slash: bool = False, +) -> tuple[str, ...]: + """ + Split a relative path and reject any segment that would change which + resource the path addresses. + + Each segment is checked *as it will decode*, because + ``_percent_encode_relative_url`` normalizes with ``unquote`` before + quoting: "%2e%2e" would otherwise pass a literal check and still reach + ``urljoin`` as "..", and "%2f" would smuggle in a separator. ``urljoin`` + resolves "." and ".." while building a request, which shifts the bucket + segment - "../../../other-bucket/x" turns a validated "skills/my-bucket" + into a request against ``other-bucket``. + + ``allow_trailing_slash`` accepts one trailing empty segment, which is how + DIAL spells "this is a folder". It is off for paths given to a resource + reference, where the terminal call decides file vs folder. + """ + if not path.strip(): + raise InvalidDialURLError(f"{param} must not be empty") + if path.startswith("/"): + raise InvalidDialURLError(f"{param} must be relative, got: {path!r}") + + segments = tuple(path.split("/")) + if not allow_trailing_slash and path.endswith("/"): + raise InvalidDialURLError( + f"{param} must not end with '/', got: {path!r}. The terminal call" + " decides whether the reference names a file or a folder." + ) + + # A trailing slash marks a folder; it is not a segment of its own. + checked = segments[:-1] if path.endswith("/") else segments + for segment in checked: + decoded = unquote(segment) + if decoded == "": + raise InvalidDialURLError( + f"Empty path segment in {param}, got: {path!r}" + ) + if decoded in (".", ".."): + raise InvalidDialURLError( + f'"." and ".." are not allowed in {param}, got: {path!r}' + ) + if "/" in decoded: + raise InvalidDialURLError( + "An encoded path separator is not allowed in" + f" {param}, got: {path!r}" + ) + return segments + + +def _storage_error_processor( + http_status_error: httpx.HTTPStatusError, +) -> DialException | None: + """ + Translate the status codes DIAL storage endpoints use for optimistic + concurrency and absence into the typed exception hierarchy. + """ + if http_status_error.response.status_code == 412: + return EtagMismatchError( + message=http_status_error.response.text, + ) + elif http_status_error.response.status_code == 404: + return ResourceNotFoundError( + message=http_status_error.response.text, + ) + return None + + def _is_directory(s: str) -> bool: - return s[-1] == "/" + return s.endswith("/") class DialStorageResource(BaseModel): - resource_type: StorageResourceType + resource_type: AnyStorageResourceType """Bucket name, like 'my-bucket'""" bucket: str @@ -47,8 +145,11 @@ class DialStorageResource(BaseModel): """Path without api prefix, like 'files/my-bucket/my-folder/my-file.txt'""" api_path: str - """Path without bucket, like my-folder/'my-file.txt'""" - bucket_path: str + """ + Path without bucket, like 'my-folder/my-file.txt' + None when the URL points at the bucket root + """ + bucket_path: str | None = None """ Filename, like 'my-file.txt' @@ -61,39 +162,64 @@ def safe_parse_storage_resource( *, url: str, dial_api_url: str, - expected_resource_type: StorageResourceType | None = None, + expected_resource_type: AnyStorageResourceType | None = None, + allow_empty_bucket_path: bool = False, ) -> DialStorageResource | NotDialURLError | InvalidDialURLError: """ Parse the storage resource from the URL, that could be 1. Absolute: "https://dial.core/v1/files/my-bucket/my-file.txt" 2. Relative to API prefix: "files/my-bucket/my-file.txt" + + ``allow_empty_bucket_path`` accepts a URL that names a bucket and nothing + inside it, like "skills/my-bucket". It is opt-in because a two-segment path + is ambiguous: "files/my-file.txt" has the same shape and is a missing-bucket + error. A resource reference passes it unconditionally - it does not yet know + which call comes next, so the endpoint's own requirements are checked when + the request is built. """ dial_api_url = enforce_trailing_slash(dial_api_url) if url.startswith("/"): return InvalidDialURLError(f"Root-relative URL is forbidden: {url}") - if url.startswith(API_PREFIX): + if url.startswith((API_PREFIX_V1, API_PREFIX_V2)): return InvalidDialURLError( f"API prefix as relative part is not allowed: {url}" ) + # Reject traversal on the raw string, before urljoin below resolves it: + # _percent_encode_relative_url leaves ".." intact (quote treats "." as + # always-safe), so urljoin would silently retarget another bucket. + url_path = urlsplit(url).path.lstrip("/") + if url_path: + try: + split_relative_segments(url_path, "url", allow_trailing_slash=True) + except InvalidDialURLError as error: + return error + absolute_url = urljoin(dial_api_url, _percent_encode_relative_url(url)) url_parsed = urlparse(absolute_url) dial_api_parsed = urlparse(dial_api_url) if url_parsed.netloc != dial_api_parsed.netloc: return NotDialURLError(message=f"Provided URL is not DIAL URL: {url}") try: - url_path = PurePosixPath(url_parsed.path) - api_path = url_path.relative_to(dial_api_parsed.path) + url_path_parsed = PurePosixPath(url_parsed.path) + api_path = url_path_parsed.relative_to(dial_api_parsed.path) except ValueError: return InvalidDialURLError( f"Provided URL path {url_parsed.path} does not match with" f" DIAL API URL {dial_api_parsed.path}" ) + # "{resource_type}/{bucket}" is the shortest addressable path. + if len(api_path.parents) < 2: + return InvalidDialURLError(f"Missing bucket in URL: {url}") + resource_path = api_path.parents[len(api_path.parents) - 2] parsed_resource_type = str(resource_path) - if parsed_resource_type not in get_args(StorageResourceType): + if parsed_resource_type not in ( + *get_args(StorageResourceTypeV1), + *get_args(StorageResourceTypeV2), + ): return InvalidDialURLError( f"Invalid resource type: {parsed_resource_type}" ) @@ -108,17 +234,28 @@ def safe_parse_storage_resource( ) if len(api_path.parents) < 3: - return InvalidDialURLError(f"Missing bucket in URL: {url}") + if not allow_empty_bucket_path: + return InvalidDialURLError(f"Missing bucket path in URL: {url}") + # The URL is "{resource_type}/{bucket}" - the bucket itself. + return DialStorageResource( + resource_type=cast(AnyStorageResourceType, parsed_resource_type), + absolute_url=absolute_url, + api_path=str(api_path), + bucket=api_path.name, + bucket_path=None, + relative_url=str(url_path_parsed), + filename=None, + ) bucket_path = api_path.parents[len(api_path.parents) - 3] return DialStorageResource( - resource_type=cast(StorageResourceType, parsed_resource_type), + resource_type=cast(AnyStorageResourceType, parsed_resource_type), absolute_url=absolute_url, api_path=str(api_path), bucket=str(bucket_path.relative_to(resource_path)), bucket_path=str(api_path.relative_to(bucket_path)), - relative_url=str(url_path), - filename=url_path.name if not _is_directory(url) else None, + relative_url=str(url_path_parsed), + filename=url_path_parsed.name if not _is_directory(url) else None, ) @@ -126,12 +263,14 @@ def parse_storage_resource( *, url: str, dial_api_url: str, - expected_resource_type: StorageResourceType | None = None, + expected_resource_type: AnyStorageResourceType | None = None, + allow_empty_bucket_path: bool = False, ) -> DialStorageResource: result = safe_parse_storage_resource( url=url, dial_api_url=dial_api_url, expected_resource_type=expected_resource_type, + allow_empty_bucket_path=allow_empty_bucket_path, ) if isinstance(result, NotDialURLError | InvalidDialURLError): raise result @@ -144,18 +283,29 @@ class DialStorageResourceMixin(BaseModel): - /v1/files - /v1/conversations - /v1/prompts + - /v2/skills """ - resource_type: StorageResourceType + resource_type: AnyStorageResourceType dial_api_url: str + def get_api_prefix(self) -> str: + """The API prefix serving this resource, implied by its type.""" + return api_prefix_for(self.resource_type) + def get_storage_resource( - self, url: str | PurePosixPath + self, + url: str | PurePosixPath, + *, + allow_empty_bucket_path: bool = False, ) -> DialStorageResource: """ Get the storage resource object from the URL Args: url (str | PurePosixPath): The URL to be processed. + allow_empty_bucket_path (bool): Accept a URL naming a bucket and + nothing inside it, such as "skills/my-bucket". Off by default, + since a two-segment path is otherwise a missing-bucket error. Returns: DialStorageResource: The storage resource object """ @@ -163,18 +313,27 @@ def get_storage_resource( url=str(url), dial_api_url=self.dial_api_url, expected_resource_type=self.resource_type, + allow_empty_bucket_path=allow_empty_bucket_path, ) - def get_api_path(self, url: str | PurePosixPath) -> str: + def get_api_path( + self, + url: str | PurePosixPath, + *, + allow_empty_bucket_path: bool = False, + ) -> str: """ Convert URL, that could relative or absolute, to relative, percent-encoded API path. """ - return self.get_storage_resource(url).api_path + return self.get_storage_resource( + url, allow_empty_bucket_path=allow_empty_bucket_path + ).api_path - def get_display_name(self, url: str | PurePosixPath) -> str: + def get_display_name(self, url: str | PurePosixPath) -> str | None: """ Get the display name of the resource from the URL + None when the URL points at the bucket root. """ return self.get_storage_resource(url).bucket_path @@ -190,7 +349,7 @@ def _prepare_download_request( options = FinalRequestOptions( method="GET", - url=urljoin(API_PREFIX, storage_resource.api_path), + url=urljoin(self.get_api_prefix(), storage_resource.api_path), headers=remove_none( { "If-Match": etag_if_match, diff --git a/aidial_client/resources/__init__.py b/aidial_client/resources/__init__.py index 170b700..8859d9f 100644 --- a/aidial_client/resources/__init__.py +++ b/aidial_client/resources/__init__.py @@ -17,6 +17,12 @@ from .chat import AsyncChat, Chat from .files import AsyncFiles, Files from .prompts import AsyncPrompts, Prompts +from .skills import ( + AsyncSkillFilesRef, + AsyncSkillsRef, + SkillFilesRef, + SkillsRef, +) __all__ = [ "Chat", @@ -27,6 +33,10 @@ "AsyncFiles", "Prompts", "AsyncPrompts", + "SkillsRef", + "AsyncSkillsRef", + "SkillFilesRef", + "AsyncSkillFilesRef", "AsyncDeployments", "Deployments", "AsyncMetadata", diff --git a/aidial_client/resources/bucket.py b/aidial_client/resources/bucket.py index caf7536..c1569d2 100644 --- a/aidial_client/resources/bucket.py +++ b/aidial_client/resources/bucket.py @@ -1,6 +1,6 @@ from urllib.parse import urljoin -from aidial_client._constants import API_PREFIX +from aidial_client._constants import API_PREFIX_V1 from aidial_client._internal_types._http_request import FinalRequestOptions from aidial_client.resources.base import AsyncResource, Resource from aidial_client.types.bucket import AppData, BucketResponse @@ -11,7 +11,7 @@ def get_raw(self) -> BucketResponse: return self.http_client.request( cast_to=BucketResponse, options=FinalRequestOptions( - method="GET", url=urljoin(API_PREFIX, "bucket") + method="GET", url=urljoin(API_PREFIX_V1, "bucket") ), ) @@ -31,7 +31,7 @@ async def get_raw(self) -> BucketResponse: return await self.http_client.request( cast_to=BucketResponse, options=FinalRequestOptions( - method="GET", url=urljoin(API_PREFIX, "bucket") + method="GET", url=urljoin(API_PREFIX_V1, "bucket") ), ) diff --git a/aidial_client/resources/files.py b/aidial_client/resources/files.py index c7133ac..be079b8 100644 --- a/aidial_client/resources/files.py +++ b/aidial_client/resources/files.py @@ -6,19 +6,18 @@ import httpx -from aidial_client._constants import API_PREFIX -from aidial_client._exception import ( - DialException, - EtagMismatchError, - ResourceNotFoundError, -) +from aidial_client._constants import API_PREFIX_V1 from aidial_client._internal_types._generic import NoneType from aidial_client._internal_types._http_request import ( FileTypes, FinalRequestOptions, ) from aidial_client._utils._dict import remove_none -from aidial_client.helpers.storage_resource import DialStorageResourceMixin +from aidial_client.helpers.storage_resource import ( + DialStorageResourceMixin, + StorageResourceTypeV1, + _storage_error_processor, +) from aidial_client.resources.base import AsyncResource, Resource from aidial_client.resources.metadata import AsyncMetadata, Metadata from aidial_client.types.file import FileDownloadResponse @@ -38,23 +37,9 @@ def _move_copy_body( } -def _files_error_processor( - http_status_error: httpx.HTTPStatusError, -) -> DialException | None: - if http_status_error.response.status_code == 412: - return EtagMismatchError( - message=http_status_error.response.text, - ) - elif http_status_error.response.status_code == 404: - return ResourceNotFoundError( - message=http_status_error.response.text, - ) - return None - - class Files(Resource, DialStorageResourceMixin): metadata: Metadata - resource_type: str = "files" + resource_type: StorageResourceTypeV1 = "files" def upload( self, @@ -67,7 +52,7 @@ def upload( cast_to=FileItem, options=FinalRequestOptions( method="PUT", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), files={"file": file}, headers=remove_none( { @@ -76,7 +61,7 @@ def upload( } ), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) def download( @@ -88,7 +73,7 @@ def download( response = self.http_client.request( cast_to=httpx.Response, options=options, - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) return FileDownloadResponse(response=response, filename=filename) @@ -101,14 +86,14 @@ def delete( cast_to=NoneType, options=FinalRequestOptions( method="DELETE", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), headers=remove_none( { "If-Match": etag_if_match, } ), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) def move_to( @@ -121,10 +106,10 @@ def move_to( cast_to=NoneType, options=FinalRequestOptions( method="POST", - url=urljoin(API_PREFIX, "ops/resource/move"), + url=urljoin(API_PREFIX_V1, "ops/resource/move"), json_data=_move_copy_body(self, source, destination, overwrite), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) def copy_to( @@ -137,10 +122,10 @@ def copy_to( cast_to=NoneType, options=FinalRequestOptions( method="POST", - url=urljoin(API_PREFIX, "ops/resource/copy"), + url=urljoin(API_PREFIX_V1, "ops/resource/copy"), json_data=_move_copy_body(self, source, destination, overwrite), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) def get_metadata( @@ -160,7 +145,7 @@ def get_metadata( class AsyncFiles(AsyncResource, DialStorageResourceMixin): metadata: AsyncMetadata - resource_type: str = "files" + resource_type: StorageResourceTypeV1 = "files" async def upload( self, @@ -173,7 +158,7 @@ async def upload( cast_to=FileItem, options=FinalRequestOptions( method="PUT", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), files={"file": file}, headers=remove_none( { @@ -182,7 +167,7 @@ async def upload( } ), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) async def download( @@ -194,7 +179,7 @@ async def download( response = await self.http_client.request( cast_to=httpx.Response, options=options, - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) return FileDownloadResponse(response=response, filename=filename) @@ -207,7 +192,7 @@ async def stream_download( options, filename = self._prepare_download_request(url, etag_if_match) async with self.http_client.stream( options=options, - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) as response: yield FileDownloadResponse(response=response, filename=filename) @@ -220,14 +205,14 @@ async def delete( cast_to=NoneType, options=FinalRequestOptions( method="DELETE", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), headers=remove_none( { "If-Match": etag_if_match, } ), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) async def move_to( @@ -240,10 +225,10 @@ async def move_to( cast_to=NoneType, options=FinalRequestOptions( method="POST", - url=urljoin(API_PREFIX, "ops/resource/move"), + url=urljoin(API_PREFIX_V1, "ops/resource/move"), json_data=_move_copy_body(self, source, destination, overwrite), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) async def copy_to( @@ -256,10 +241,10 @@ async def copy_to( cast_to=NoneType, options=FinalRequestOptions( method="POST", - url=urljoin(API_PREFIX, "ops/resource/copy"), + url=urljoin(API_PREFIX_V1, "ops/resource/copy"), json_data=_move_copy_body(self, source, destination, overwrite), ), - on_http_error=_files_error_processor, + on_http_error=_storage_error_processor, ) async def get_metadata( diff --git a/aidial_client/resources/metadata.py b/aidial_client/resources/metadata.py index 0e12f16..799c3af 100644 --- a/aidial_client/resources/metadata.py +++ b/aidial_client/resources/metadata.py @@ -3,11 +3,11 @@ from typing_extensions import assert_never -from aidial_client._constants import METADATA_PREFIX +from aidial_client._constants import METADATA_PREFIX_V1 from aidial_client._internal_types._http_request import FinalRequestOptions from aidial_client._utils._dict import remove_none from aidial_client.helpers.storage_resource import ( - StorageResourceType, + StorageResourceTypeV1, _percent_encode_relative_url, ) from aidial_client.resources.base import AsyncResource, Resource @@ -19,7 +19,7 @@ def _get_cast_to( - resource: StorageResourceType, + resource: StorageResourceTypeV1, ) -> type[FileMetadata] | type[ConversationMetadata] | type[PromptMetadata]: if resource == "files": return FileMetadata @@ -64,7 +64,7 @@ def get( def get( self, - resource: StorageResourceType, + resource: StorageResourceTypeV1, relative_url: str, *, limit: int | None = None, @@ -75,7 +75,7 @@ def get( options=FinalRequestOptions( method="GET", url=urljoin( - METADATA_PREFIX, + METADATA_PREFIX_V1, _percent_encode_relative_url(relative_url), ), params=remove_none({"limit": limit, "token": token}), @@ -116,7 +116,7 @@ async def get( async def get( self, - resource: StorageResourceType, + resource: StorageResourceTypeV1, relative_url: str, *, limit: int | None = None, @@ -127,7 +127,7 @@ async def get( options=FinalRequestOptions( method="GET", url=urljoin( - METADATA_PREFIX, + METADATA_PREFIX_V1, _percent_encode_relative_url(relative_url), ), params=remove_none({"limit": limit, "token": token}), diff --git a/aidial_client/resources/prompts.py b/aidial_client/resources/prompts.py index 7aa6955..fc18db7 100644 --- a/aidial_client/resources/prompts.py +++ b/aidial_client/resources/prompts.py @@ -2,39 +2,22 @@ from typing import Any, Literal from urllib.parse import urljoin -import httpx - from aidial_client._compatibility.pydantic import PYDANTIC_V2 -from aidial_client._constants import API_PREFIX -from aidial_client._exception import ( - DialException, - EtagMismatchError, - ResourceNotFoundError, -) +from aidial_client._constants import API_PREFIX_V1 from aidial_client._internal_types._generic import NoneType from aidial_client._internal_types._http_request import FinalRequestOptions from aidial_client._utils._dict import remove_none -from aidial_client.helpers.storage_resource import DialStorageResourceMixin +from aidial_client.helpers.storage_resource import ( + DialStorageResourceMixin, + StorageResourceTypeV1, + _storage_error_processor, +) from aidial_client.resources.base import AsyncResource, Resource from aidial_client.resources.metadata import AsyncMetadata, Metadata from aidial_client.types.metadata import PromptItem, PromptMetadata from aidial_client.types.prompt import Prompt -def _prompts_error_processor( - http_status_error: httpx.HTTPStatusError, -) -> DialException | None: - if http_status_error.response.status_code == 412: - return EtagMismatchError( - message=http_status_error.response.text, - ) - elif http_status_error.response.status_code == 404: - return ResourceNotFoundError( - message=http_status_error.response.text, - ) - return None - - def _prompt_to_json(prompt: Prompt) -> dict[str, Any]: if PYDANTIC_V2: return prompt.model_dump(by_alias=True) # type: ignore @@ -43,7 +26,7 @@ def _prompt_to_json(prompt: Prompt) -> dict[str, Any]: class Prompts(Resource, DialStorageResourceMixin): metadata: Metadata - resource_type: str = "prompts" + resource_type: StorageResourceTypeV1 = "prompts" def save( self, @@ -56,7 +39,7 @@ def save( cast_to=PromptItem, options=FinalRequestOptions( method="PUT", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), json_data=_prompt_to_json(prompt), headers=remove_none( { @@ -65,7 +48,7 @@ def save( } ), ), - on_http_error=_prompts_error_processor, + on_http_error=_storage_error_processor, ) def get(self, url: str | PurePosixPath) -> Prompt: @@ -74,9 +57,9 @@ def get(self, url: str | PurePosixPath) -> Prompt: cast_to=Prompt, options=FinalRequestOptions( method="GET", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), ), - on_http_error=_prompts_error_processor, + on_http_error=_storage_error_processor, ) def delete( @@ -88,14 +71,14 @@ def delete( cast_to=NoneType, options=FinalRequestOptions( method="DELETE", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), headers=remove_none( { "If-Match": etag_if_match, } ), ), - on_http_error=_prompts_error_processor, + on_http_error=_storage_error_processor, ) def get_metadata(self, url: str | PurePosixPath) -> PromptMetadata: @@ -107,7 +90,7 @@ def get_metadata(self, url: str | PurePosixPath) -> PromptMetadata: class AsyncPrompts(AsyncResource, DialStorageResourceMixin): metadata: AsyncMetadata - resource_type: str = "prompts" + resource_type: StorageResourceTypeV1 = "prompts" async def save( self, @@ -120,7 +103,7 @@ async def save( cast_to=PromptItem, options=FinalRequestOptions( method="PUT", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), json_data=_prompt_to_json(prompt), headers=remove_none( { @@ -129,7 +112,7 @@ async def save( } ), ), - on_http_error=_prompts_error_processor, + on_http_error=_storage_error_processor, ) async def get(self, url: str | PurePosixPath) -> Prompt: @@ -138,9 +121,9 @@ async def get(self, url: str | PurePosixPath) -> Prompt: cast_to=Prompt, options=FinalRequestOptions( method="GET", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), ), - on_http_error=_prompts_error_processor, + on_http_error=_storage_error_processor, ) async def delete( @@ -152,14 +135,14 @@ async def delete( cast_to=NoneType, options=FinalRequestOptions( method="DELETE", - url=urljoin(API_PREFIX, self.get_api_path(url)), + url=urljoin(API_PREFIX_V1, self.get_api_path(url)), headers=remove_none( { "If-Match": etag_if_match, } ), ), - on_http_error=_prompts_error_processor, + on_http_error=_storage_error_processor, ) async def get_metadata(self, url: str | PurePosixPath) -> PromptMetadata: diff --git a/aidial_client/resources/skills.py b/aidial_client/resources/skills.py new file mode 100644 index 0000000..205462a --- /dev/null +++ b/aidial_client/resources/skills.py @@ -0,0 +1,495 @@ +""" +Chained references to DIAL Core's ``/v2/skills`` API. + +A skill is a folder-shaped resource: the whole skill is addressed as a unit at +"skills/{bucket}/{path}", and its bundled files hang off +"skills/{bucket}/{path}/files/{filePath}". + +Rather than taking a hand-built URL per call, the resource is a reference that +is narrowed step by step - ``client.skills / "writing" / "tone-of-voice"`` - +so the URL is assembled from validated segments and cannot be typed wrong. +Each narrowing returns a new reference; references are immutable and issue no +requests until a terminal call (``list``/``read``/``download``/``stream``). + +The invariant that splits the two kinds of error: **constructing a reference +validates segment syntax; a terminal call validates that the reference has +enough path for its route.** +""" + +from collections.abc import AsyncIterator, Awaitable, Callable, Sequence +from contextlib import asynccontextmanager +from urllib.parse import unquote + +import httpx +from typing_extensions import Self, overload + +from aidial_client._constants import API_PREFIX_V2, METADATA_PREFIX_V2 +from aidial_client._exception import InvalidDialURLError +from aidial_client._internal_types._http_request import FinalRequestOptions +from aidial_client._utils._dict import remove_none +from aidial_client.helpers.storage_resource import ( + DialStorageResourceMixin, + StorageResourceTypeV2, + _percent_encode_relative_url, + _storage_error_processor, + split_relative_segments, +) +from aidial_client.resources.base import AsyncResource, Resource +from aidial_client.types.file import FileDownloadResponse +from aidial_client.types.metadata import SkillFileMetadata, SkillMetadata + +# DIAL Core reserves this path segment to keep the +# ".../{path}/files/{filePath}" grammar unambiguous. +_FILES_SEGMENT = "files" + + +def _encode(segments: Sequence[str]) -> str: + """Percent-encode each segment, so none can contribute a separator.""" + return "/".join(_percent_encode_relative_url(seg) for seg in segments) + + +def _listing_params( + limit: int | None, + token: str | None, + recursive: bool | None, +) -> dict[str, object]: + return remove_none({"limit": limit, "token": token, "recursive": recursive}) + + +def _validate_bucket(bucket: str) -> str: + """A bucket is a URL segment too, so it gets the same treatment.""" + segments = split_relative_segments(bucket, "bucket") + if len(segments) != 1: + raise InvalidDialURLError( + f"bucket must be a single path segment, got: {bucket!r}" + ) + return segments[0] + + +class _RefCommon(DialStorageResourceMixin): + """State and URL parsing shared by both kinds of reference.""" + + resource_type: StorageResourceTypeV2 = "skills" + bucket: str | None = None + """None means "the caller's own bucket", resolved at terminal-call time.""" + + def _split_url(self, url: str) -> tuple[str, tuple[str, ...]]: + # allow_empty_bucket_path is unconditional: a reference does not know + # which terminal call comes next, so "skills/my-bucket" must parse and + # the shape guards decide later whether an empty path is acceptable. + parsed = self.get_storage_resource(url, allow_empty_bucket_path=True) + if parsed.bucket_path is None: + return parsed.bucket, () + return parsed.bucket, tuple(parsed.bucket_path.rstrip("/").split("/")) + + +class _SkillsRefBase(_RefCommon): + """A skill, a grouping folder, or the bucket root.""" + + path: tuple[str, ...] = () + + @overload + def __call__(self, *, path: str, bucket: str | None = None) -> Self: ... + @overload + def __call__(self, *, bucket: str) -> Self: ... + @overload + def __call__(self, *, url: str) -> Self: ... + + def __call__( + self, + *, + path: str | None = None, + bucket: str | None = None, + url: str | None = None, + ) -> Self: + if url is not None: + if path is not None or bucket is not None: + raise TypeError("url= cannot be combined with bucket= or path=") + new_bucket, segments = self._split_url(url) + return self.copy(update={"bucket": new_bucket, "path": segments}) + if path is None and bucket is None: + raise TypeError("one of url=, bucket= or path= is required") + + update: dict[str, object] = {} + if bucket is not None: + update["bucket"] = _validate_bucket(bucket) + if path is not None: + update["path"] = ( + *self.path, + *split_relative_segments(path, "path"), + ) + return self.copy(update=update) + + def __truediv__(self, path: str) -> Self: + return self(path=path) + + def __repr__(self) -> str: + bucket = self.bucket if self.bucket is not None else "" + return f"{type(self).__name__}('skills/{bucket}/{'/'.join(self.path)}')" + + def _require_skill_path(self, operation: str) -> None: + if not self.path: + raise InvalidDialURLError( + f"{operation} addresses one skill, but this reference points" + " at the bucket root. Descend to a skill first, e.g." + ' client.skills / "my-skill".' + ) + + def _metadata_url(self, bucket: str) -> str: + """``GET /v2/metadata/skills/{bucket}/{path}/`` - a folder listing. + + The separator after ``{bucket}`` in Core's route regex is literal, so + an empty ``{path}`` only matches with the trailing slash. Core strips a + trailing slash off ``{path}`` again, so appending it unconditionally + leaves deeper paths resolving to the same folder as before. + """ + encoded = _encode(self.path) + suffix = f"{encoded}/" if encoded else "" + return f"{METADATA_PREFIX_V2}skills/{bucket}/{suffix}" + + def _archive_url(self, bucket: str) -> tuple[FinalRequestOptions, str]: + """``GET /v2/skills/{bucket}/{path}`` - the skill as a ZIP archive. + + Core answers ``application/zip`` without a ``Content-Disposition`` + header, so name the archive after the skill. + """ + options = FinalRequestOptions( + method="GET", + url=f"{API_PREFIX_V2}skills/{bucket}/{_encode(self.path)}", + ) + return options, f"{unquote(self.path[-1])}.zip" + + +class _SkillFilesRefBase(_RefCommon): + """The files bundled inside one skill, or a subfolder of them.""" + + skill_path: tuple[str, ...] = () + path: tuple[str, ...] = () + + @overload + def __call__(self, *, path: str) -> Self: ... + @overload + def __call__(self, *, url: str) -> Self: ... + + def __call__( + self, + *, + path: str | None = None, + url: str | None = None, + ) -> Self: + if url is not None: + if path is not None: + raise TypeError("url= cannot be combined with path=") + bucket, skill_path, file_path = self._split_files_url(url) + return self.copy( + update={ + "bucket": bucket, + "skill_path": skill_path, + "path": file_path, + } + ) + if path is None: + raise TypeError("one of url= or path= is required") + return self.copy( + update={ + "path": (*self.path, *split_relative_segments(path, "path")) + } + ) + + def __truediv__(self, path: str) -> Self: + return self(path=path) + + def __repr__(self) -> str: + bucket = self.bucket if self.bucket is not None else "" + skill = "/".join(self.skill_path) + return ( + f"{type(self).__name__}('skills/{bucket}/{skill}" + f"/{_FILES_SEGMENT}/{'/'.join(self.path)}')" + ) + + def _split_files_url( + self, url: str + ) -> tuple[str, tuple[str, ...], tuple[str, ...]]: + """Split ``skills/{bucket}/{path}/files/{filePath}`` back into parts. + + The search starts at index 1 because Core's route requires at least + one segment before ``files`` (``(?.+?)/files/``), so a skill + named "files" resolves the same way here as it does there. + """ + bucket, segments = self._split_url(url) + try: + index = segments.index(_FILES_SEGMENT, 1) + except ValueError: + raise InvalidDialURLError( + f'url must address a file inside a skill ("…/{_FILES_SEGMENT}' + f'/…"), got: {url!r}' + ) from None + return bucket, segments[:index], segments[index + 1 :] + + def _require_skill_path(self, operation: str) -> None: + if not self.skill_path: + raise InvalidDialURLError( + f"{operation} lists the files of one skill, but this" + " reference points at the bucket root. Descend to a skill" + ' first, e.g. (client.skills / "my-skill").files.' + ) + + def _require_file_path(self, operation: str) -> None: + self._require_skill_path(operation) + if not self.path: + raise InvalidDialURLError( + f"{operation} addresses one file, but no file path was given." + ' Use skill.files(path="SKILL.md") to name it.' + ) + + def _files_metadata_url(self, bucket: str) -> str: + """``GET /v2/metadata/skills/{b}/{p}/files[/{filePath}]``.""" + base = ( + f"{METADATA_PREFIX_V2}skills/{bucket}" + f"/{_encode(self.skill_path)}/{_FILES_SEGMENT}" + ) + tail = _encode(self.path) + return f"{base}/{tail}" if tail else base + + def _file_url(self, bucket: str) -> tuple[FinalRequestOptions, str]: + """``GET /v2/skills/{b}/{p}/files/{filePath}`` - one bundled file.""" + options = FinalRequestOptions( + method="GET", + url=( + f"{API_PREFIX_V2}skills/{bucket}" + f"/{_encode(self.skill_path)}/{_FILES_SEGMENT}" + f"/{_encode(self.path)}" + ), + ) + # The path is percent-encoded; return a human-readable filename. + return options, unquote(self.path[-1]) + + +class SkillsRef(Resource, _SkillsRefBase): + class Config: + arbitrary_types_allowed = True + allow_mutation = False + + resolve_bucket: Callable[[], str] + + def _bucket(self) -> str: + if self.bucket is not None: + return self.bucket + return self.resolve_bucket() + + @property + def files(self) -> "SkillFilesRef": + return SkillFilesRef( + http_client=self.http_client, + dial_api_url=self.dial_api_url, + bucket=self.bucket, + skill_path=self.path, + resolve_bucket=self.resolve_bucket, + ) + + def list( + self, + *, + limit: int | None = None, + token: str | None = None, + recursive: bool | None = None, + ) -> SkillMetadata: + """ + List the skills and grouping folders this reference points at. + + Follow ``next_token`` until it is ``None`` to read every page. + """ + return self.http_client.request( + cast_to=SkillMetadata, + options=FinalRequestOptions( + method="GET", + url=self._metadata_url(self._bucket()), + params=_listing_params(limit, token, recursive), + ), + on_http_error=_storage_error_processor, + ) + + def download(self) -> FileDownloadResponse: + """Download the whole skill as a ZIP archive.""" + self._require_skill_path("download()") + options, filename = self._archive_url(self._bucket()) + response = self.http_client.request( + cast_to=httpx.Response, + options=options, + on_http_error=_storage_error_processor, + ) + return FileDownloadResponse(response=response, filename=filename) + + +class SkillFilesRef(Resource, _SkillFilesRefBase): + class Config: + arbitrary_types_allowed = True + allow_mutation = False + + resolve_bucket: Callable[[], str] + + def _bucket(self) -> str: + if self.bucket is not None: + return self.bucket + return self.resolve_bucket() + + def list( + self, + *, + limit: int | None = None, + token: str | None = None, + recursive: bool | None = None, + ) -> SkillFileMetadata: + """ + List the skill's files, optionally scoped to a subfolder. + + A page may hold fewer entries than ``limit``, so follow ``next_token`` + until it is ``None`` rather than assuming one page is complete. + """ + self._require_skill_path("list()") + return self.http_client.request( + cast_to=SkillFileMetadata, + options=FinalRequestOptions( + method="GET", + url=self._files_metadata_url(self._bucket()), + params=_listing_params(limit, token, recursive), + ), + on_http_error=_storage_error_processor, + ) + + def read(self) -> FileDownloadResponse: + """Download the single file this reference names.""" + self._require_file_path("read()") + options, filename = self._file_url(self._bucket()) + response = self.http_client.request( + cast_to=httpx.Response, + options=options, + on_http_error=_storage_error_processor, + ) + return FileDownloadResponse(response=response, filename=filename) + + +class AsyncSkillsRef(AsyncResource, _SkillsRefBase): + class Config: + arbitrary_types_allowed = True + allow_mutation = False + + resolve_bucket: Callable[[], Awaitable[str]] + + async def _bucket(self) -> str: + if self.bucket is not None: + return self.bucket + return await self.resolve_bucket() + + @property + def files(self) -> "AsyncSkillFilesRef": + return AsyncSkillFilesRef( + http_client=self.http_client, + dial_api_url=self.dial_api_url, + bucket=self.bucket, + skill_path=self.path, + resolve_bucket=self.resolve_bucket, + ) + + async def list( + self, + *, + limit: int | None = None, + token: str | None = None, + recursive: bool | None = None, + ) -> SkillMetadata: + """ + List the skills and grouping folders this reference points at. + + Follow ``next_token`` until it is ``None`` to read every page. + """ + return await self.http_client.request( + cast_to=SkillMetadata, + options=FinalRequestOptions( + method="GET", + url=self._metadata_url(await self._bucket()), + params=_listing_params(limit, token, recursive), + ), + on_http_error=_storage_error_processor, + ) + + async def download(self) -> FileDownloadResponse: + """Download the whole skill as a ZIP archive.""" + self._require_skill_path("download()") + options, filename = self._archive_url(await self._bucket()) + response = await self.http_client.request( + cast_to=httpx.Response, + options=options, + on_http_error=_storage_error_processor, + ) + return FileDownloadResponse(response=response, filename=filename) + + @asynccontextmanager + async def stream_download(self) -> AsyncIterator[FileDownloadResponse]: + """Stream the whole skill as a ZIP archive.""" + self._require_skill_path("stream_download()") + options, filename = self._archive_url(await self._bucket()) + async with self.http_client.stream( + options=options, + on_http_error=_storage_error_processor, + ) as response: + yield FileDownloadResponse(response=response, filename=filename) + + +class AsyncSkillFilesRef(AsyncResource, _SkillFilesRefBase): + class Config: + arbitrary_types_allowed = True + allow_mutation = False + + resolve_bucket: Callable[[], Awaitable[str]] + + async def _bucket(self) -> str: + if self.bucket is not None: + return self.bucket + return await self.resolve_bucket() + + async def list( + self, + *, + limit: int | None = None, + token: str | None = None, + recursive: bool | None = None, + ) -> SkillFileMetadata: + """ + List the skill's files, optionally scoped to a subfolder. + + A page may hold fewer entries than ``limit``, so follow ``next_token`` + until it is ``None`` rather than assuming one page is complete. + """ + self._require_skill_path("list()") + return await self.http_client.request( + cast_to=SkillFileMetadata, + options=FinalRequestOptions( + method="GET", + url=self._files_metadata_url(await self._bucket()), + params=_listing_params(limit, token, recursive), + ), + on_http_error=_storage_error_processor, + ) + + async def read(self) -> FileDownloadResponse: + """Download the single file this reference names.""" + self._require_file_path("read()") + options, filename = self._file_url(await self._bucket()) + response = await self.http_client.request( + cast_to=httpx.Response, + options=options, + on_http_error=_storage_error_processor, + ) + return FileDownloadResponse(response=response, filename=filename) + + @asynccontextmanager + async def stream(self) -> AsyncIterator[FileDownloadResponse]: + """Stream the single file this reference names.""" + self._require_file_path("stream()") + options, filename = self._file_url(await self._bucket()) + async with self.http_client.stream( + options=options, + on_http_error=_storage_error_processor, + ) as response: + yield FileDownloadResponse(response=response, filename=filename) diff --git a/aidial_client/types/metadata.py b/aidial_client/types/metadata.py index d188d87..9980e91 100644 --- a/aidial_client/types/metadata.py +++ b/aidial_client/types/metadata.py @@ -22,7 +22,7 @@ class Config: bucket: str url: str node_type: Literal["FOLDER", "ITEM"] - resource_type: Literal["FILE", "CONVERSATION", "PROMPT"] + resource_type: Literal["FILE", "CONVERSATION", "PROMPT", "SKILL"] class ResourceItemMetadata(BaseMetadata): @@ -69,3 +69,40 @@ class PromptMetadata(BaseMetadata): next_token: str | None = None items: list[PromptItem] | None resource_type: Literal["PROMPT"] + + +class SkillItem(ResourceItemMetadata): + """A node in the skills listing: a skill (ITEM) or a grouping folder.""" + + node_type: Literal["FOLDER", "ITEM"] + resource_type: Literal["SKILL"] + + +class SkillMetadata(BaseMetadata): + node_type: Literal["FOLDER", "ITEM"] + resource_type: Literal["SKILL"] + next_token: str | None = None + items: list[SkillItem] | None = None + + +class SkillFileItem(ResourceItemMetadata): + """ + A file (ITEM) or a subfolder (FOLDER) inside a skill. + + A recursive listing is flattened and contains no subfolder entries at + all, so the two kinds only ever appear together in a non-recursive one. + + Sparser than the /v1 files listing: no ``content_length``, no + ``content_type``, and in observed responses no ``etag`` either. + Subfolder entries carry no timestamps. + """ + + node_type: Literal["FOLDER", "ITEM"] + resource_type: Literal["SKILL"] + + +class SkillFileMetadata(BaseMetadata): + node_type: Literal["FOLDER", "ITEM"] + resource_type: Literal["SKILL"] + next_token: str | None = None + items: list[SkillFileItem] | None = None diff --git a/tests/helpers/test_storage_resource_mixin.py b/tests/helpers/test_storage_resource_mixin.py index a12ce0c..f5d3a2d 100644 --- a/tests/helpers/test_storage_resource_mixin.py +++ b/tests/helpers/test_storage_resource_mixin.py @@ -104,7 +104,7 @@ def test_get_api_path_missing_bucket(resource_type, url): mixin = DialStorageResourceMixin( resource_type=resource_type, dial_api_url=DIAL_API_URL ) - with pytest.raises(InvalidDialURLError, match="Missing bucket in URL"): + with pytest.raises(InvalidDialURLError, match="Missing bucket path in URL"): mixin.get_api_path(url) diff --git a/tests/helpers/test_storage_resource_parser.py b/tests/helpers/test_storage_resource_parser.py index dba29a2..c198610 100644 --- a/tests/helpers/test_storage_resource_parser.py +++ b/tests/helpers/test_storage_resource_parser.py @@ -178,3 +178,179 @@ def test_parse_storage_resource_non_dial_ignore(): dial_api_url="https://dial.core/v1/", expected_resource_type="files", ) + + +@pytest.mark.parametrize( + "url, expected_api_path", + [ + ("skills/my-bucket/my-skill", "skills/my-bucket/my-skill"), + ("skills/my-bucket/group/my-skill", "skills/my-bucket/group/my-skill"), + ( + "https://dial.core/v2/skills/my-bucket/my-skill", + "skills/my-bucket/my-skill", + ), + ], +) +def test_parse_v2_skill_resource(url, expected_api_path): + result = parse_storage_resource( + url=url, + dial_api_url="https://dial.core/v2/", + expected_resource_type="skills", + ) + assert result.resource_type == "skills" + assert result.bucket == "my-bucket" + assert result.api_path == expected_api_path + + +@pytest.mark.parametrize( + "url, dial_api_url, resource_type", + [ + ("skills/my-bucket", "https://dial.core/v2/", "skills"), + ("skills/my-bucket/", "https://dial.core/v2/", "skills"), + ("files/my-bucket", "https://dial.core/v1/", "files"), + ], +) +def test_parse_bucket_root_when_allowed(url, dial_api_url, resource_type): + result = parse_storage_resource( + url=url, + dial_api_url=dial_api_url, + expected_resource_type=resource_type, + allow_empty_bucket_path=True, + ) + assert result.bucket == "my-bucket" + assert result.bucket_path is None + assert result.filename is None + assert result.api_path == f"{resource_type}/my-bucket" + + +@pytest.mark.parametrize( + "url, dial_api_url, resource_type", + [ + ("skills/my-bucket", "https://dial.core/v2/", "skills"), + ("files/my-bucket", "https://dial.core/v1/", "files"), + ], +) +def test_parse_bucket_root_rejected_by_default( + url, dial_api_url, resource_type +): + # A two-segment path is ambiguous ("files/my-file.txt" has the same + # shape), so bucket-root parsing stays opt-in. + with pytest.raises(InvalidDialURLError, match="Missing bucket path in URL"): + parse_storage_resource( + url=url, + dial_api_url=dial_api_url, + expected_resource_type=resource_type, + ) + + +def test_parse_rejects_v2_api_prefix_as_relative_part(): + with pytest.raises( + InvalidDialURLError, match="API prefix as relative part" + ): + parse_storage_resource( + url="v2/skills/my-bucket/my-skill", + dial_api_url="https://dial.core/v2/", + expected_resource_type="skills", + ) + + +def test_parse_rejects_skills_url_for_v1_resource(): + with pytest.raises(InvalidDialURLError, match="Invalid resource type"): + parse_storage_resource( + url="skills/my-bucket/my-skill", + dial_api_url="https://dial.core/v1/", + expected_resource_type="files", + ) + + +@pytest.mark.parametrize( + "url, expected_bucket, expected_bucket_path", + [ + # The urls apps actually ship in their "skills" config: the shared + # public bucket, a nested grouping folder, and a generated bucket id. + # Core's bucket group is [a-zA-Z0-9]+, which both bucket forms satisfy. + ( + "skills/public/demo/azure-resource-visualizer", + "public", + "demo/azure-resource-visualizer", + ), + ( + "skills/public/all-three-conventionss", + "public", + "all-three-conventionss", + ), + ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/all-three-conventionss", + "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "all-three-conventionss", + ), + ], +) +def test_parse_real_world_skill_urls( + url, expected_bucket, expected_bucket_path +): + result = parse_storage_resource( + url=url, + dial_api_url="https://dial.core/v2/", + expected_resource_type="skills", + ) + assert result.bucket == expected_bucket + assert result.bucket_path == expected_bucket_path + + +@pytest.mark.parametrize( + "url, dial_api_url, resource_type", + [ + # urljoin resolves ".." while building the request, which silently + # retargets another bucket. The parser must reject it first - and it + # does so for /v1 resources too, not just skills. + ("files/my-bucket/../../other/x.txt", "https://dial.core/v1/", "files"), + ("files/my-bucket/../other/x.txt", "https://dial.core/v1/", "files"), + ("prompts/my-bucket/./p.txt", "https://dial.core/v1/", "prompts"), + ("skills/my-bucket/../other/skill", "https://dial.core/v2/", "skills"), + # Checked as it decodes: quote() leaves ".." intact, so a literal + # check on the raw string would miss these. + ( + "skills/my-bucket/%2e%2e/%2e%2e/other/skill", + "https://dial.core/v2/", + "skills", + ), + ("skills/my-bucket/.%2e/other", "https://dial.core/v2/", "skills"), + # An encoded separator would smuggle in an extra path segment. + ("skills/my-bucket/a%2fb", "https://dial.core/v2/", "skills"), + # Interior empty segment. + ("files/my-bucket//x.txt", "https://dial.core/v1/", "files"), + ( + "https://dial.core/v1/files/my-bucket/../other/x.txt", + "https://dial.core/v1/", + "files", + ), + ], +) +def test_parse_rejects_path_traversal(url, dial_api_url, resource_type): + with pytest.raises(InvalidDialURLError): + parse_storage_resource( + url=url, + dial_api_url=dial_api_url, + expected_resource_type=resource_type, + ) + + +@pytest.mark.parametrize( + "url", + [ + # A trailing slash is how DIAL spells "folder" - it must survive. + "files/my-bucket/relative_folder/", + "files/my-bucket/a/deep/folder/", + "https://dial.core/v1/files/my-bucket/relative_folder/", + ], +) +def test_parse_keeps_accepting_folder_urls(url): + result = parse_storage_resource( + url=url, + dial_api_url="https://dial.core/v1/", + expected_resource_type="files", + ) + assert result.bucket == "my-bucket" + assert result.filename is None diff --git a/tests/resources/skills/test_skill_core_routes.py b/tests/resources/skills/test_skill_core_routes.py new file mode 100644 index 0000000..8b7761e --- /dev/null +++ b/tests/resources/skills/test_skill_core_routes.py @@ -0,0 +1,142 @@ +""" +Pin the URLs the skills resource builds against DIAL Core's own route +regexes. + +The rest of the skills tests assert URL strings, which cannot catch a URL +that is well-formed but unroutable - a bucket-root listing without its +trailing slash matched every string assertion while Core would have answered +404. These patterns are copied verbatim from ai-dial-core's +``server/.../data/RouteTemplate.java``; keep them in sync when Core changes +them. + +The bucket here is alphanumeric on purpose: Core's ``[a-zA-Z0-9]+`` bucket +group rejects the hyphenated names used elsewhere in these tests, so a +hyphenated fixture would fail to match for the wrong reason. +""" + +import re +from collections.abc import Callable +from typing import Any +from unittest.mock import Mock + +import httpx +import pytest + +from aidial_client import Dial + +COMPLEX_RESOURCE = re.compile( + r"^/v2/skills/(?P[a-zA-Z0-9]+)" + r"/(?P[^/](?:[^/]|/(?=[^/])(?!files/))*)$" +) +COMPLEX_RESOURCE_FILE = re.compile( + r"^/v2/skills/(?P[a-zA-Z0-9]+)" + r"/(?P.+?)/files/(?P.+)$" +) +COMPLEX_RESOURCE_FILE_METADATA = re.compile( + r"^/v2/metadata/skills/(?P[a-zA-Z0-9]+)" + r"/(?P.+?)/files(?:/(?P.*))?$" +) +COMPLEX_RESOURCE_METADATA = re.compile( + r"^/v2/metadata/skills/(?P[a-zA-Z0-9]+)/(?P.*)$" +) + +BUCKET = "mybucket7a1" +SKILL = f"skills/{BUCKET}/writing/toneofvoice" + + +# Parses as both SkillMetadata and SkillFileMetadata; the binary reads cast +# to httpx.Response and ignore it. Only the request URL matters here. +EMPTY_LISTING = { + "bucket": BUCKET, + "url": f"skills/{BUCKET}/", + "nodeType": "FOLDER", + "resourceType": "SKILL", +} + + +def _client(captured: list[httpx.Request]) -> Dial: + client = Dial(api_key="dummy", base_url="http://dial.core") + + def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response( + status_code=200, request=request, json=EMPTY_LISTING + ) + response.request = request + return response + + client._http_client._internal_http_client.send = send_mock + client._get_my_bucket = Mock(return_value=BUCKET) + return client + + +def _route( + call: Callable[[Dial], object], +) -> tuple[str, dict[str, str | None]]: + captured: list[httpx.Request] = [] + call(_client(captured)) + path = captured[0].url.raw_path.decode() + + # Core checks the file routes before the whole-resource ones, so a path + # carrying a "/files/" segment must be reported as the file route. + for name, pattern in ( + ("COMPLEX_RESOURCE_FILE_METADATA", COMPLEX_RESOURCE_FILE_METADATA), + ("COMPLEX_RESOURCE_METADATA", COMPLEX_RESOURCE_METADATA), + ("COMPLEX_RESOURCE_FILE", COMPLEX_RESOURCE_FILE), + ("COMPLEX_RESOURCE", COMPLEX_RESOURCE), + ): + match = pattern.match(path) + if match: + return name, match.groupdict() + pytest.fail(f"{path} matches no /v2/skills route in DIAL Core") + + +def test_bucket_root_listing_is_routable(): + route, groups = _route(lambda client: client.skills.list()) + + assert route == "COMPLEX_RESOURCE_METADATA" + assert groups["bucket"] == BUCKET + # An empty {path} is how Core lists the bucket root - and the separator + # before it is literal, so this only matches with the trailing slash. + assert groups["path"] == "" + + +def test_grouping_folder_listing_is_routable(): + route, groups = _route(lambda client: (client.skills / "writing").list()) + + assert route == "COMPLEX_RESOURCE_METADATA" + assert groups["path"] == "writing/" + + +def test_list_files_is_routable(): + route, groups = _route(lambda client: client.skills(url=SKILL).files.list()) + + assert route == "COMPLEX_RESOURCE_FILE_METADATA" + assert groups["path"] == "writing/toneofvoice" + assert groups["filePath"] is None + + +def test_list_files_subfolder_is_routable(): + route, groups = _route( + lambda client: client.skills(url=SKILL).files(path="references").list() + ) + + assert route == "COMPLEX_RESOURCE_FILE_METADATA" + assert groups["filePath"] == "references" + + +def test_get_file_is_routable(): + route, groups = _route( + lambda client: client.skills(url=SKILL).files(path="SKILL.md").read() + ) + + assert route == "COMPLEX_RESOURCE_FILE" + assert groups["path"] == "writing/toneofvoice" + assert groups["filePath"] == "SKILL.md" + + +def test_download_is_routable(): + route, groups = _route(lambda client: client.skills(url=SKILL).download()) + + assert route == "COMPLEX_RESOURCE" + assert groups["path"] == "writing/toneofvoice" diff --git a/tests/resources/skills/test_skill_download.py b/tests/resources/skills/test_skill_download.py new file mode 100644 index 0000000..f1646cf --- /dev/null +++ b/tests/resources/skills/test_skill_download.py @@ -0,0 +1,353 @@ +from typing import Any, cast +from unittest.mock import AsyncMock + +import httpx +import pytest + +from aidial_client import Dial +from aidial_client._client import AsyncDial +from aidial_client._exception import ( + DialException, + EtagMismatchError, + InvalidDialURLError, + ResourceNotFoundError, +) +from tests.client_mock import MockStreamIterator, get_client_mock + +SKILL_URL = "skills/test-bucket/writing/tone-of-voice" +ZIP_BYTES = b"PK\x03\x04fake-archive" + + +def _capturing_client( + captured: list[httpx.Request], + content: bytes, + headers: dict[str, str] | None = None, +) -> Dial: + client = Dial(api_key="dummy", base_url="http://dial.core") + + def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response( + status_code=200, + request=request, + content=content, + headers=headers or {}, + ) + response.request = request + return response + + client._http_client._internal_http_client.send = send_mock + return client + + +def test_download_whole_skill_as_zip(): + captured: list[httpx.Request] = [] + client = _capturing_client( + captured, + ZIP_BYTES, + {"content-type": "application/zip", "etag": "aggregate-etag"}, + ) + + response = client.skills(url=SKILL_URL).download() + + assert ( + captured[0].url.path == "/v2/skills/test-bucket/writing/tone-of-voice" + ) + assert response.get_content() == ZIP_BYTES + assert response.content_type == "application/zip" + assert response.headers["etag"] == "aggregate-etag" + # Core sends no Content-Disposition, so the archive is named after the skill. + assert response.filename == "tone-of-voice.zip" + + +def test_reads_send_no_if_match(): + # DIAL Core ignores If-Match on both v2 reads: neither + # ComplexResourceController.get nor .getFile calls ProxyUtil.etag, and + # neither operation declares the header or a 412 response. Sending it + # anyway would advertise a precondition the server does not enforce. + captured: list[httpx.Request] = [] + client = _capturing_client(captured, ZIP_BYTES) + + client.skills(url=SKILL_URL).download() + client.skills(url=SKILL_URL).files(path="SKILL.md").read() + + assert all("if-match" not in request.headers for request in captured) + + +def test_download_rejects_non_skill_url(): + client = _capturing_client([], ZIP_BYTES) + + with pytest.raises(InvalidDialURLError, match="Invalid resource type"): + client.skills(url="files/test-bucket/folder/file.txt") + + +def test_get_file_returns_bytes(): + captured: list[httpx.Request] = [] + client = _capturing_client( + captured, + b"---\nname: tone\ndescription: d\n---\nbody", + {"content-type": "text/markdown", "etag": "aggregate-etag"}, + ) + + response = client.skills(url=SKILL_URL).files(path="SKILL.md").read() + + assert captured[0].url.path == ( + "/v2/skills/test-bucket/writing/tone-of-voice/files/SKILL.md" + ) + assert response.filename == "SKILL.md" + assert response.get_content().startswith(b"---") + + +def test_get_file_percent_encodes_relative_path(): + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"schema") + + response = ( + client.skills(url=SKILL_URL) + .files(path="references/api schema.md") + .read() + ) + + assert captured[0].url.raw_path.decode() == ( + "/v2/skills/test-bucket/writing/tone-of-voice" + "/files/references/api%20schema.md" + ) + # The filename stays human-readable. + assert response.filename == "api schema.md" + + +def test_get_file_accepts_already_encoded_path(): + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"schema") + + client.skills(url=SKILL_URL).files(path="references/api%20schema.md").read() + + assert captured[0].url.raw_path.decode() == ( + "/v2/skills/test-bucket/writing/tone-of-voice" + "/files/references/api%20schema.md" + ) + + +def test_get_file_preserves_non_utf8_content(): + payload = b"\x89PNG\r\n\x1a\n\xff\xfe" + client = _capturing_client([], payload, {"content-type": "image/png"}) + + response = client.skills(url=SKILL_URL).files(path="assets/logo.png").read() + + assert response.get_content() == payload + + +@pytest.mark.parametrize( + "status_code, expected_exception", + [ + (404, ResourceNotFoundError), + (412, EtagMismatchError), + (403, DialException), + ], +) +def test_error_mapping(status_code, expected_exception): + client = get_client_mock( + status_code=status_code, json_mock={"error": {"message": "nope"}} + ) + + with pytest.raises(expected_exception) as exc_info: + client.skills(url=SKILL_URL).files(path="SKILL.md").read() + + if status_code == 403: + assert exc_info.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_async_stream_download_streams_and_closes(): + captured: list[httpx.Request] = [] + captured_kwargs: list[dict[str, Any]] = [] + responses: list[httpx.Response] = [] + client = AsyncDial(api_key="dummy", base_url="http://dial.core") + client._get_my_bucket = cast(Any, AsyncMock(return_value="test-bucket")) + + async def send_mock( + request: httpx.Request, *, stream: bool = False, **kwargs: Any + ) -> httpx.Response: + captured.append(request) + captured_kwargs.append({"stream": stream, **kwargs}) + response = httpx.Response( + status_code=200, + request=request, + stream=MockStreamIterator(mock_chunks=[b"PK\x03\x04", b"rest"]), + ) + responses.append(response) + return response + + client._http_client._internal_http_client.send = cast(Any, send_mock) + + async with client.skills(url=SKILL_URL).stream_download() as response: + assert response.filename == "tone-of-voice.zip" + chunks = [chunk async for chunk in response] + assert b"".join(chunks) == b"PK\x03\x04rest" + + assert ( + captured[0].url.path == "/v2/skills/test-bucket/writing/tone-of-voice" + ) + assert captured_kwargs == [{"stream": True}] + assert responses[0].is_closed is True + + +@pytest.mark.asyncio +async def test_async_stream_file_streams_and_closes(): + captured: list[httpx.Request] = [] + responses: list[httpx.Response] = [] + client = AsyncDial(api_key="dummy", base_url="http://dial.core") + + async def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response( + status_code=200, + request=request, + stream=MockStreamIterator(mock_chunks=[b"# skill"]), + ) + responses.append(response) + return response + + client._http_client._internal_http_client.send = cast(Any, send_mock) + + async with ( + client.skills(url=SKILL_URL).files(path="SKILL.md").stream() as response + ): + assert response.filename == "SKILL.md" + assert b"".join([c async for c in response]) == b"# skill" + + assert captured[0].url.path == ( + "/v2/skills/test-bucket/writing/tone-of-voice/files/SKILL.md" + ) + assert responses[0].is_closed is True + + +@pytest.mark.parametrize( + "bad_path", + [ + "../../../victimbucket9f2/their-skill/files/SKILL.md", + "../SKILL.md", + "refs/../../../other/x.md", + "refs/./x.md", + ".", + "..", + # Percent-encoded dot segments: _percent_encode_relative_url + # unquotes before quoting, so these decode back to ".." and would + # otherwise slip past a literal check. + "%2e%2e/%2e%2e/%2e%2e/victimbucket9f2/s/files/SKILL.md", + "%2E%2E/SKILL.md", + "refs/%2e/x.md", + ".%2e/SKILL.md", + ], +) +def test_get_file_rejects_traversal_segments(bad_path): + # file_path is appended to an already-validated api path and never goes + # back through the url parser, so "." / ".." would shift the bucket + # segment and retarget the request at another bucket. + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"x") + + with pytest.raises(InvalidDialURLError, match=r'"\." and "\.\."'): + client.skills(url=SKILL_URL).files(path=bad_path).read() + + assert captured == [] + + +@pytest.mark.parametrize( + "bad_path", + [ + "../../../victimbucket9f2/their-skill/files", + "refs/../../..", + "%2e%2e/%2e%2e/%2e%2e/victimbucket9f2/s/files", + ], +) +def test_list_files_rejects_traversal_segments(bad_path): + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"x") + + with pytest.raises(InvalidDialURLError, match=r'"\." and "\.\."'): + client.skills(url=SKILL_URL).files(path=bad_path).list() + + assert captured == [] + + +@pytest.mark.parametrize( + "bad_path, message", + [ + ("", "must not be empty"), + (" ", "must not be empty"), + ("/abs/path.md", "must be relative"), + ("refs/", "must not end with"), + ("a//b.md", "Empty path segment"), + ], +) +def test_get_file_rejects_malformed_path(bad_path, message): + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"x") + + with pytest.raises(InvalidDialURLError, match=message): + client.skills(url=SKILL_URL).files(path=bad_path).read() + + assert captured == [] + + +@pytest.mark.asyncio +async def test_async_get_file_rejects_traversal(): + # The reference is built synchronously, so a bad segment is rejected + # before anything is awaited - there is no request to intercept. + client = AsyncDial(api_key="dummy", base_url="http://dial.core") + + with pytest.raises(InvalidDialURLError, match=r'"\." and "\.\."'): + client.skills(url=SKILL_URL).files(path="../../../other/s/files/x.md") + + +@pytest.mark.parametrize( + "bad_path", + ["refs/a%2Fb.md", "a%2fb.md", "a%2f%2e%2e%2fb"], +) +def test_get_file_rejects_encoded_separator(bad_path): + # An encoded separator would decode into a real one, splitting a segment + # after validation and landing in the derived filename. + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"x") + + with pytest.raises(InvalidDialURLError, match="encoded path separator"): + client.skills(url=SKILL_URL).files(path=bad_path).read() + + assert captured == [] + + +def test_rejects_empty_path(): + # Omitting path= keeps the reference where it is; passing "" is an error, + # not a way to say "no path". + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"x") + + with pytest.raises(InvalidDialURLError, match="path must not be empty"): + client.skills(url=SKILL_URL).files(path="") + + assert captured == [] + + +@pytest.mark.parametrize( + "good_path, expected_raw, expected_filename", + [ + ("SKILL.md", "SKILL.md", "SKILL.md"), + ("refs/api schema.md", "refs/api%20schema.md", "api schema.md"), + ("refs/api%20schema.md", "refs/api%20schema.md", "api schema.md"), + # A dot inside a segment is not a dot segment. + ("v1.2/notes.md", "v1.2/notes.md", "notes.md"), + ], +) +def test_get_file_accepts_legitimate_paths( + good_path, expected_raw, expected_filename +): + captured: list[httpx.Request] = [] + client = _capturing_client(captured, b"x") + + response = client.skills(url=SKILL_URL).files(path=good_path).read() + + assert captured[0].url.raw_path.decode() == ( + f"/v2/skills/test-bucket/writing/tone-of-voice/files/{expected_raw}" + ) + assert response.filename == expected_filename diff --git a/tests/resources/skills/test_skill_metadata.py b/tests/resources/skills/test_skill_metadata.py new file mode 100644 index 0000000..40b45e8 --- /dev/null +++ b/tests/resources/skills/test_skill_metadata.py @@ -0,0 +1,280 @@ +from typing import Any, cast +from unittest.mock import AsyncMock, Mock + +import httpx +import pytest + +from aidial_client import Dial +from aidial_client._client import AsyncDial +from aidial_client._exception import InvalidDialURLError + +SKILLS_LISTING_MOCK = { + "name": "writing", + "parentPath": None, + "bucket": "test-bucket", + "url": "skills/test-bucket/writing/", + "nodeType": "FOLDER", + "resourceType": "SKILL", + "items": [ + { + "name": "tone-of-voice", + "parentPath": "writing", + "bucket": "test-bucket", + "url": "skills/test-bucket/writing/tone-of-voice", + "nodeType": "ITEM", + "resourceType": "SKILL", + "createdAt": 1700000000000, + "updatedAt": 1700000001000, + "author": "someone", + }, + { + "name": "drafts", + "parentPath": "writing", + "bucket": "test-bucket", + "url": "skills/test-bucket/writing/drafts/", + "nodeType": "FOLDER", + "resourceType": "SKILL", + }, + ], + "nextToken": "next-page-token", +} + +SKILL_FILES_MOCK = { + "name": "files", + "parentPath": "tone-of-voice", + "bucket": "test-bucket", + "url": "skills/test-bucket/tone-of-voice/files/", + "nodeType": "FOLDER", + "resourceType": "SKILL", + "items": [ + { + "name": "SKILL.md", + "parentPath": "tone-of-voice/files", + "bucket": "test-bucket", + "url": "skills/test-bucket/tone-of-voice/files/SKILL.md", + "nodeType": "ITEM", + "resourceType": "SKILL", + # Observed responses carry no etag on these entries at all; + # this pins that one is parsed when a deployment does send it. + "etag": "abc123", + "updatedAt": 1700000001000, + }, + { + "name": "references", + "parentPath": "tone-of-voice/files", + "bucket": "test-bucket", + # A subfolder, as a non-recursive listing reports one. + "url": "skills/test-bucket/tone-of-voice/files/references/", + "nodeType": "FOLDER", + "resourceType": "SKILL", + }, + ], + "nextToken": None, +} + + +def _sync_client(captured: list[httpx.Request], payload: dict) -> Dial: + client = Dial(api_key="dummy", base_url="http://dial.core") + + def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response( + status_code=200, request=request, json=payload + ) + response.request = request + return response + + client._http_client._internal_http_client.send = send_mock + client._get_my_bucket = Mock(return_value="test-bucket") + return client + + +def _async_client(captured: list[httpx.Request], payload: dict) -> AsyncDial: + client = AsyncDial(api_key="dummy", base_url="http://dial.core") + + async def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response( + status_code=200, request=request, json=payload + ) + response.request = request + return response + + client._http_client._internal_http_client.send = cast(Any, send_mock) + client._get_my_bucket = cast(Any, AsyncMock(return_value="test-bucket")) + return client + + +def test_get_metadata_lists_bucket_root(): + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILLS_LISTING_MOCK) + + result = client.skills.list() + + assert captured[0].url.path == "/v2/metadata/skills/test-bucket/" + assert result.resource_type == "SKILL" + assert result.next_token == "next-page-token" # noqa: S105 + + items = result.items or [] + assert [item.node_type for item in items] == ["ITEM", "FOLDER"] + # A skill node is an ITEM; Core omits the aggregate etag from the listing. + assert items[0].name == "tone-of-voice" + assert items[0].etag is None + assert items[0].author == "someone" + + +def test_get_metadata_passes_listing_params(): + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILLS_LISTING_MOCK) + + (client.skills / "writing").list( + limit=1000, + token="page-2", # noqa: S106 + recursive=True, + ) + + request = captured[0] + assert request.url.path == "/v2/metadata/skills/test-bucket/writing/" + assert dict(request.url.params) == { + "limit": "1000", + "token": "page-2", # noqa: S105 + "recursive": "true", + } + + +def test_get_metadata_omits_unset_params(): + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILLS_LISTING_MOCK) + + (client.skills / "writing").list() + + assert dict(captured[0].url.params) == {} + + +def test_list_files_defaults_to_skill_root(): + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILL_FILES_MOCK) + + result = client.skills(url="skills/test-bucket/tone-of-voice").files.list( + recursive=True, limit=1000 + ) + + request = captured[0] + assert ( + request.url.path + == "/v2/metadata/skills/test-bucket/tone-of-voice/files" + ) + assert dict(request.url.params) == {"limit": "1000", "recursive": "true"} + + items = result.items or [] + assert items[0].etag == "abc123" + assert result.next_token is None + + # A subfolder is a FOLDER and carries a trailing "/" on its url. + assert [item.node_type for item in items] == ["ITEM", "FOLDER"] + assert [item.url.endswith("/") for item in items] == [False, True] + + +def test_list_files_scopes_to_subfolder(): + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILL_FILES_MOCK) + + client.skills(url="skills/test-bucket/tone-of-voice").files( + path="references/api schema" + ).list() + + assert captured[0].url.raw_path.decode() == ( + "/v2/metadata/skills/test-bucket/tone-of-voice" + "/files/references/api%20schema" + ) + + +def test_list_files_pagination_loop_terminates(): + pages = [ + {**SKILL_FILES_MOCK, "nextToken": "page-2"}, + {**SKILL_FILES_MOCK, "nextToken": None}, + ] + captured: list[httpx.Request] = [] + client = Dial(api_key="dummy", base_url="http://dial.core") + + def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + response = httpx.Response( + status_code=200, + request=request, + json=pages[len(captured)], + ) + captured.append(request) + response.request = request + return response + + client._http_client._internal_http_client.send = send_mock + + files = client.skills(url="skills/test-bucket/tone-of-voice").files + + token = None + seen = 0 + while True: + page = files.list(token=token) + seen += len(page.items or []) + token = page.next_token + if token is None: + break + + assert len(captured) == 2 + assert seen == 4 + assert dict(captured[1].url.params) == {"token": "page-2"} + + +def test_get_metadata_rejects_non_skill_url(): + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILLS_LISTING_MOCK) + + # Rejected while the reference is built, so nothing reaches the wire. + with pytest.raises(InvalidDialURLError, match="Invalid resource type"): + client.skills(url="files/test-bucket/folder") + + assert captured == [] + + +def test_list_files_rejects_bucket_root(): + # A bucket has no files of its own - only skills do. The reference is + # legal; the terminal call is what has too little path for its route. + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILL_FILES_MOCK) + + with pytest.raises(InvalidDialURLError, match="points at the bucket root"): + client.skills(url="skills/test-bucket").files.list() + + assert captured == [] + + +@pytest.mark.asyncio +async def test_async_get_metadata_and_list_files(): + captured: list[httpx.Request] = [] + client = _async_client(captured, SKILLS_LISTING_MOCK) + + result = await client.skills.list() + assert captured[0].url.path == "/v2/metadata/skills/test-bucket/" + assert (result.items or [])[0].name == "tone-of-voice" + + await client.skills(url="skills/test-bucket/tone-of-voice").files.list( + recursive=True + ) + assert ( + captured[1].url.path + == "/v2/metadata/skills/test-bucket/tone-of-voice/files" + ) + + +def test_rejects_folder_path_with_trailing_slash(): + # A reference carries no trailing slash: list() vs read() is what decides + # whether it names a folder or a file. The old list_files(path="refs/") + # accepted this and dropped the slash silently. + captured: list[httpx.Request] = [] + client = _sync_client(captured, SKILL_FILES_MOCK) + + skill = client.skills(url="skills/test-bucket/tone-of-voice") + with pytest.raises(InvalidDialURLError, match="must not end with"): + skill.files(path="references/") + + assert captured == [] diff --git a/tests/resources/skills/test_skill_refs.py b/tests/resources/skills/test_skill_refs.py new file mode 100644 index 0000000..273f8f3 --- /dev/null +++ b/tests/resources/skills/test_skill_refs.py @@ -0,0 +1,450 @@ +""" +What the chained-reference design itself adds, on top of the URLs the other +skills tests pin: equivalence of the ways to aim a reference, immutability, +laziness of the bucket lookup, and segment validation at construction time. +""" + +from typing import Any, cast +from unittest.mock import AsyncMock, Mock + +import httpx +import pytest + +from aidial_client import AsyncDial, Dial +from aidial_client._exception import InvalidDialURLError +from aidial_client.types.metadata import SkillFileMetadata + +BUCKET = "test-bucket" +SKILL_URL = f"skills/{BUCKET}/writing/tone-of-voice" +LISTING: dict[str, Any] = { + "bucket": BUCKET, + "url": f"skills/{BUCKET}/", + "nodeType": "FOLDER", + "resourceType": "SKILL", +} + + +def _client(captured: list[httpx.Request]) -> Dial: + client = Dial(api_key="dummy", base_url="http://dial.core") + + def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response(200, request=request, json=LISTING) + response.request = request + return response + + client._http_client._internal_http_client.send = send_mock + client._get_my_bucket = Mock(return_value=BUCKET) + return client + + +def _async_client(captured: list[httpx.Request]) -> AsyncDial: + client = AsyncDial(api_key="dummy", base_url="http://dial.core") + + async def send_mock(request: httpx.Request, **_: Any) -> httpx.Response: + captured.append(request) + response = httpx.Response(200, request=request, json=LISTING) + response.request = request + return response + + client._http_client._internal_http_client.send = cast(Any, send_mock) + client._get_my_bucket = cast(Any, AsyncMock(return_value=BUCKET)) + return client + + +def _path_of(build) -> str: + captured: list[httpx.Request] = [] + build(_client(captured)) + return captured[0].url.raw_path.decode() + + +# --- aiming a reference -------------------------------------------------- + + +def test_truediv_and_path_are_equivalent(): + by_operator = _path_of(lambda c: (c.skills / "a" / "b").list()) + by_keyword = _path_of(lambda c: c.skills(path="a/b").list()) + by_mixture = _path_of(lambda c: (c.skills / "a")(path="b").list()) + + assert by_operator == f"/v2/metadata/skills/{BUCKET}/a/b/" + assert by_operator == by_keyword == by_mixture + + +def test_path_descends_and_bucket_is_kept(): + assert ( + _path_of(lambda c: (c.skills / "writing")(bucket="public").list()) + == "/v2/metadata/skills/public/writing/" + ) + + +def test_url_replaces_both_bucket_and_path(): + assert ( + _path_of( + lambda c: c.skills(bucket="other", path="ignored")( + url="skills/elsewhere/deep/skill" + ).download() + ) + == "/v2/skills/elsewhere/deep/skill" + ) + + +def test_files_url_round_trips_a_listing_entry(): + # An entry of a files listing carries the "/files/" segment, so it cannot + # go back through skills(url=...) - the files reference parses it instead. + assert ( + _path_of( + lambda c: c.skills.files( + url=f"{SKILL_URL}/files/references/api.md" + ).read() + ) + == f"/v2/skills/{BUCKET}/writing/tone-of-voice/files/references/api.md" + ) + + +def test_files_url_splits_after_the_first_segment(): + # Core's route requires at least one segment before "files" + # ("(?.+?)/files/"), so a skill named "files" resolves the same way. + assert ( + _path_of( + lambda c: c.skills.files( + url=f"skills/{BUCKET}/files/files/a.md" + ).read() + ) + == f"/v2/skills/{BUCKET}/files/files/a.md" + ) + + +def test_files_url_without_a_files_segment_is_rejected(): + client = _client([]) + + with pytest.raises(InvalidDialURLError, match="must address a file"): + client.skills.files(url=SKILL_URL) + + +@pytest.mark.parametrize( + "call, message", + [ + (lambda ref: ref(url="x", bucket="b"), "cannot be combined"), + (lambda ref: ref(url="x", path="p"), "cannot be combined"), + (lambda ref: ref(), "is required"), + ], +) +def test_bad_argument_combinations_are_type_errors(call, message): + # The overloads already reject these statically, so reaching them is a + # programming error rather than a bad URL. + client = _client([]) + + with pytest.raises(TypeError, match=message): + call(client.skills) + + +# --- immutability and laziness ------------------------------------------- + + +def test_narrowing_leaves_the_original_untouched(): + captured: list[httpx.Request] = [] + client = _client(captured) + + root = client.skills + sub = root / "writing" + + assert root is not sub + assert root.path == () + assert sub.path == ("writing",) + + root.list() + assert captured[0].url.path == f"/v2/metadata/skills/{BUCKET}/" + + +@pytest.mark.parametrize( + "build", + [ + lambda c: c.skills, + lambda c: c.skills.files, + lambda c: _async_client([]).skills, + lambda c: _async_client([]).skills.files, + ], +) +def test_references_are_immutable(build): + reference = build(_client([])) + + with pytest.raises(TypeError): + reference.path = ("mutated",) + + +def test_building_a_reference_issues_no_requests(): + captured: list[httpx.Request] = [] + client = _client(captured) + + chain = (client.skills / "writing" / "tone-of-voice").files(path="refs") + + assert captured == [] + assert client._get_my_bucket.call_count == 0 + assert chain.skill_path == ("writing", "tone-of-voice") + + +def test_own_bucket_is_resolved_once_and_shared(): + client = _client([]) + + client.skills.list() + (client.skills / "writing").list() + + assert client._get_my_bucket.call_count == 1 + + +@pytest.mark.parametrize( + "build", + [ + lambda c: c.skills(bucket="public").list(), + lambda c: c.skills(url=SKILL_URL).download(), + ], +) +def test_explicit_bucket_skips_the_lookup(build): + client = _client([]) + + build(client) + + assert client._get_my_bucket.call_count == 0 + + +# --- shape guards run before any I/O ------------------------------------- + + +@pytest.mark.parametrize( + "call, message", + [ + (lambda c: c.skills.download(), "points at the bucket root"), + (lambda c: c.skills.files.list(), "points at the bucket root"), + ( + lambda c: c.skills(url=SKILL_URL).files.read(), + "no file path was given", + ), + ], +) +def test_shape_guards_precede_bucket_resolution(call, message): + captured: list[httpx.Request] = [] + client = _client(captured) + + with pytest.raises(InvalidDialURLError, match=message): + call(client) + + assert captured == [] + # The guard has to come first, or a doomed call still costs a round-trip. + assert client._get_my_bucket.call_count == 0 + + +# --- segment validation -------------------------------------------------- + + +@pytest.mark.parametrize( + "bad", + ["", " ", "/abs", "a//b", ".", "..", "%2e%2e", ".%2e", "a%2fb", "refs/"], +) +@pytest.mark.parametrize( + "aim", + [ + lambda ref, value: ref(path=value), + lambda ref, value: ref / value, + ], +) +def test_every_path_surface_validates_segments(aim, bad): + client = _client([]) + + with pytest.raises(InvalidDialURLError): + aim(client.skills, bad) + with pytest.raises(InvalidDialURLError): + aim(client.skills(url=SKILL_URL).files, bad) + + +@pytest.mark.parametrize("bad", ["", "a/b", "..", "a%2fb"]) +def test_bucket_is_validated_as_a_single_segment(bad): + client = _client([]) + + with pytest.raises(InvalidDialURLError): + client.skills(bucket=bad) + + +def test_root_reference_has_no_phantom_dot_segment(): + # PurePosixPath("") is PurePosixPath("."), which would put a "." in the + # URL. Segments are stored as a tuple precisely to avoid that. + client = _client([]) + + assert client.skills.path == () + assert _path_of(lambda c: c.skills.list()).endswith(f"/{BUCKET}/") + + +# --- filenames ----------------------------------------------------------- + + +def test_archive_is_named_after_the_skill(): + client = _client([]) + + assert client.skills(url=SKILL_URL).download().filename == ( + "tone-of-voice.zip" + ) + + +def test_read_returns_a_human_readable_filename(): + client = _client([]) + + response = ( + client.skills(url=SKILL_URL).files(path="refs/api%20schema.md").read() + ) + + assert response.filename == "api schema.md" + + +# --- async mirror -------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_async_chain_builds_without_awaiting(): + captured: list[httpx.Request] = [] + client = _async_client(captured) + + skill = client.skills / "writing" / "tone-of-voice" + files = skill.files(path="references") + assert captured == [] + + await files.list(recursive=True) + + assert captured[0].url.path == ( + f"/v2/metadata/skills/{BUCKET}/writing/tone-of-voice/files/references" + ) + assert client._get_my_bucket.await_count == 1 + + +# --- listing shapes ------------------------------------------------------- + +# Real listings captured from DIAL Core (2026-09-04), one per mode. A +# recursive listing flattens the tree to leaf files at every depth; a +# non-recursive one returns the immediate children. Neither carries an etag. +REAL_RECURSIVE_LISTING: dict[str, Any] = { + "name": "files", + "parentPath": "all-three-conventionss", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/all-three-conventionss/files/" + ), + "nodeType": "FOLDER", + "resourceType": "SKILL", + "items": [ + { + "name": "SKILL.md", + "parentPath": "all-three-conventionss/files", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/all-three-conventionss/files/SKILL.md" + ), + "nodeType": "ITEM", + "resourceType": "SKILL", + "updatedAt": 1788267212576, + }, + { + "name": "regions.csv", + "parentPath": "all-three-conventionss/files/assets", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/all-three-conventionss/files/assets/regions.csv" + ), + "nodeType": "ITEM", + "resourceType": "SKILL", + "updatedAt": 1788267212609, + }, + { + "name": "extract.py", + "parentPath": "all-three-conventionss/files/scripts", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/all-three-conventionss/files/scripts/extract.py" + ), + "nodeType": "ITEM", + "resourceType": "SKILL", + "updatedAt": 1788267212641, + }, + ], +} + +# The same skill listed non-recursively: files and directories side by side. +# The directory nodeTypes here are post-fix (epam/ai-dial-core#1912) - the +# original capture reported them as "ITEM", which is what the client used to +# correct. Directories still carry no timestamps: that branch copies none. +REAL_NON_RECURSIVE_LISTING: dict[str, Any] = { + "name": "files", + "parentPath": "skill-creator", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/skill-creator/files/" + ), + "nodeType": "FOLDER", + "resourceType": "SKILL", + "items": [ + { + "name": "SKILL.md", + "parentPath": "skill-creator/files", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/skill-creator/files/SKILL.md" + ), + "nodeType": "ITEM", + "resourceType": "SKILL", + "updatedAt": 1788530552986, + }, + { + "name": "agents", + "parentPath": "skill-creator/files", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/skill-creator/files/agents/" + ), + "nodeType": "FOLDER", + "resourceType": "SKILL", + }, + { + "name": "scripts", + "parentPath": "skill-creator/files", + "bucket": "4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc", + "url": ( + "skills/4T56XoBkFtVqFQFmwHtbkUbjx8zLC8Sypb3xrJH4MACc" + "/skill-creator/files/scripts/" + ), + "nodeType": "FOLDER", + "resourceType": "SKILL", + }, + ], +} + + +def test_recursive_listing_is_flat(): + page = SkillFileMetadata(**REAL_RECURSIVE_LISTING) + + # Leaf files at every depth, no folder entries, structure in parent_path. + assert page.node_type == "FOLDER" + assert [i.node_type for i in page.items or []] == ["ITEM"] * 3 + assert [i.name for i in page.items or []] == [ + "SKILL.md", + "regions.csv", + "extract.py", + ] + + +def test_non_recursive_listing_carries_subfolders(): + page = SkillFileMetadata(**REAL_NON_RECURSIVE_LISTING) + items = page.items or [] + + # node_type comes straight from the response; nothing is rewritten. + assert [(i.name, i.node_type) for i in items] == [ + ("SKILL.md", "ITEM"), + ("agents", "FOLDER"), + ("scripts", "FOLDER"), + ] + # Subfolder entries carry no timestamp, and nothing here carries an etag. + assert [i.updated_at for i in items] == [1788530552986, None, None] + assert all(i.etag is None for i in items)