From 47a2a4ab0e8ee63eb82bafdd3c870558dd5f4de9 Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Tue, 25 Aug 2026 15:57:40 -0400 Subject: [PATCH] =?UTF-8?q?feat(client):=20Agent=20Skills=20=E2=80=94=20re?= =?UTF-8?q?ference=20discovery?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First of five slices splitting the Agent Skills feature for review. This one adds the layer with no I/O in it at all: the types, the validation, and the projection from a resolved AI Config to the skills it references. - `skill_refs(config)` projects a config's `skills` array into `list[SkillReference]`. Pure — no client, no store, no network, no telemetry. - `Skill` and `SkillReference` are frozen dataclasses, exported from the package root. `Skill.content` is `bytes` — the verified verbatim bytes LaunchDarkly delivered, exactly what was hashed. Skills are opaque byte buffers by construction: the SDK never parses, decodes, or interprets skill content anywhere. `content_hash` is the sha256 (lowercase hex) over those bytes, and the optional display metadata comes from LaunchDarkly, never from the content. - `parse_ai_config` now validates the optional `skills` array and fails closed on a malformed one. Key grammar, length bound, and the version predicate live in `types_validation.py` as one canonical rejection reason, so every layer added on top rejects a key for the same stated reason. Note one behaviour change for existing users: `parse_ai_config` fails closed on a `skills` value that is not a list of `{key, version}` objects, where before any value parsed and was ignored. A variation carrying its own differently shaped `skills` field must rename it before upgrading. The two layers that follow — retrieval through a store seam, and materialization onto disk — are separate slices. `agents.md` names all three and the one-way dependencies between them. Testing: `uv run pytest` → 1048 passed. `ruff check`, `ruff format --check`, and `mypy packages/*/src` all clean. Co-Authored-By: Claude Fable 5 --- packages/client/README.md | 41 ++++++ packages/client/agents.md | 79 +++++++++- .../src/launchdarkly_ai_server/__init__.py | 9 ++ .../src/launchdarkly_ai_server/skills.py | 72 +++++++++ .../src/launchdarkly_ai_server/types.py | 42 +++++- .../types_validation.py | 76 +++++++++- packages/client/tests/test_schema.py | 103 ++++++++++++- packages/client/tests/test_skills.py | 139 ++++++++++++++++++ uv.lock | 16 +- 9 files changed, 562 insertions(+), 15 deletions(-) create mode 100644 packages/client/src/launchdarkly_ai_server/skills.py create mode 100644 packages/client/tests/test_skills.py diff --git a/packages/client/README.md b/packages/client/README.md index 8691babd..fbd40045 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -220,6 +220,45 @@ asyncio.run(main()) --- +### Agent Skills + +Skills are versioned `SKILL.md` documents managed in LaunchDarkly and attached to AI Config +variations by reference. This release adds the first layer: discovering which skills a +resolved config references. Retrieving their content and materializing them onto disk follow. + +```python +import asyncio + +from launchdarkly_ai_server import init_client, inspect_config, skill_refs + +async def main(): + await init_client() + + info = await inspect_config("doc-agent", {"kind": "user", "key": "user-123"}) + refs = skill_refs(info["config"]) # [SkillReference(key='pdf-extraction', version=2)] + + for ref in refs: + print(ref.key, ref.version) + +asyncio.run(main()) +``` + +**`skills` is now a validated field.** Config parsing fails closed on a `skills` value that +is not a list of `{key, version}` objects (key matching `^[a-z0-9][a-z0-9-]*$`, version an +integer ≥ 1): the whole variation is rejected, `inspect_config` returns `config: None`, and +`extract_variation` raises. A variation that previously carried its own custom `skills` +field of a different shape must rename it before upgrading. + +| Export | Description | +|---|---| +| `skill_refs(config)` | Project a config's `skills` array into `list[SkillReference]`. Pure — no client, store, or network needed. Returns `[]` when absent. | + +> `Skill.content` is `bytes` — the verified verbatim bytes LaunchDarkly delivered, exactly +> what was hashed. The SDK never parses or interprets them; if you want the frontmatter, +> decode and parse the content on your side. + +--- + ### Utility Helpers ```python @@ -250,3 +289,5 @@ All types are exported from this package. Handler packages import them from here | `GraphNode` / `GraphEdge` | A dataclass node (`.key`, `.config`, `.meta`, `.edges`, `.is_terminal`) and a dataclass directed edge (`.key`, `.source_key`, `.target_key`, `.handoff`) | | `ProviderGraphResponse` | A dataclass returned by `graph(...).invoke()`: `.response`, `.usage`, `.judge_results` | | `GraphTopology` | The parsed graph flag shape (`root` + `edges`) | +| `Skill` | A frozen skill document: `.key`, `.version`, `.content` (verified verbatim `bytes`), `.content_hash`, `.name?`, `.description?` | +| `SkillReference` | A frozen version-pinned pointer to a skill: `.key`, `.version` | diff --git a/packages/client/agents.md b/packages/client/agents.md index 61d99cf0..8b55f173 100644 --- a/packages/client/agents.md +++ b/packages/client/agents.md @@ -27,7 +27,8 @@ No other `launchdarkly-ai-*` package may define or duplicate these. They import | `src/launchdarkly_ai_server/tracking.py` | `execute_and_track`, `execute_and_stream`, `wrap_tool_handlers`, `parse_usage` | | `src/launchdarkly_ai_server/graph.py` | `graph()`, `resolve_graph()`, `GraphInstance` | | `src/launchdarkly_ai_server/types.py` | All shared Python types — `AiConfigRep`, `ProviderHandler`, `LDContext`, `NativeTool`, etc. | -| `src/launchdarkly_ai_server/types_validation.py` | `parse_ai_config` — validates flag variation shape | +| `src/launchdarkly_ai_server/types_validation.py` | `parse_ai_config` — validates flag variation shape; `is_valid_skill_key` / `is_valid_skill_version` / `skill_key_rejection_reason` (the canonical key-grammar explanation every layer quotes) | +| `src/launchdarkly_ai_server/skills.py` | Agent Skills — `skill_refs`, the projection of a config's `skills` array into typed references | | `src/launchdarkly_ai_server/utils.py` | `parse_template`, `parse_json_with_possible_fences`, `create_handler`, `parse_usage`, `make_track_data`, `to_ld_context` | | `src/launchdarkly_ai_server/registry.py` | `Registry`, `global_registry`, `compose`, `resolve_handlers`, `resolve_tools` | | `src/launchdarkly_ai_server/judges.py` | `run_judges`, `build_judge_tasks`, `run_judge` | @@ -53,6 +54,7 @@ from launchdarkly_ai_server import ( TrackData, UsageDict, HandlerResult, HandlerStreamEvent, StreamEvent, StreamChunkEvent, StreamDoneEvent, ExecuteStreamEvent, ExecuteStreamDoneEvent, VariationMeta, InitClientOptions, JudgeResult, ParseResult, ParseSuccess, ParseFailure, + Skill, SkillReference, ) # Utilities @@ -69,6 +71,9 @@ from launchdarkly_ai_server import execute_and_track, execute_and_stream, wrap_t # Entry points from launchdarkly_ai_server import config, graph, resolve_graph + +# Agent Skills +from launchdarkly_ai_server import skill_refs ``` When adding a new export, add it to `__init__.py`'s imports and `__all__`. Handler packages must never import from sub-paths (e.g. `launchdarkly_ai_server.client`). @@ -155,6 +160,32 @@ This is an OTel context value, not W3C baggage, so the id does not leak onto out --- +## Agent Skills + +Versioned `SKILL.md` documents attached to AI Config variations by reference, retrieved +through an injectable store, and materialized onto disk for agent runtimes to discover. +Three layers, in increasing order of blast radius. Only the first is implemented here: + +1. **Reference discovery** — `skill_refs(config)` projects the config's `skills` array into + typed `SkillReference` values. Pure: no network, no client, no store, no telemetry. + Validation of the array itself lives in `parse_ai_config` and is **fail closed** — one + malformed reference fails the whole config parse. +2. **Content accessors** — reading skill content through a store seam. +3. **Materialization** — writing skills onto disk under a manifest. + +### Security posture — do not relax any of this + +- **Skill content is an opaque byte buffer.** `Skill.content` is `bytes` — the verified + verbatim bytes LaunchDarkly delivered, exactly what was hashed. This SDK never parses, + decodes, or interprets them anywhere: not in the integrity path, not in an accessor, not + during materialization. Consumers who want frontmatter parse it themselves. +- **A key is untrusted input everywhere it appears.** `skill_key_rejection_reason` is the + single canonical explanation, so the config parser and the reference projection reject a + key for the same stated reason — and so does every layer added later. A silently + shortened projection is not acceptable: every dropped entry is logged. + +--- + ## OTel Setup The core client owns all OTel initialization. `init_client()` configures a `TracerProvider` with `ConversationIdSpanProcessor` and a `BatchSpanProcessor` plus an OTLP HTTP exporter when the optional OTel packages are installed. @@ -230,6 +261,43 @@ When `enabled` is `False`, `config` is always `None`. When `enabled` is `True` b --- +## Dependencies + +Tier 0, so the runtime surface is deliberately tiny: **one** hard dependency, and everything else either an optional extra, resolved dynamically at runtime, or dev-only. Nothing here may grow without a reason recorded in this table. + +### Runtime (`[project] dependencies`) + +| Package | Why | +|---|---| +| `opentelemetry-api>=1.25` | The tracer/span API used on every instrumented path (`tracking.py`, `graph.py`, `content.py`, `conversation.py`, `utils.py`). API-only — the *SDK* half is an optional extra, so a consumer that never configures OTel gets no-op spans rather than an `ImportError`. `conversation.py` imports `opentelemetry.sdk.trace.SpanProcessor` under `TYPE_CHECKING` only, for exactly this reason. | + +There is deliberately **no** `python-dotenv` here: `lifecycle.py` reads `os.environ` directly, so loading a `.env` file is the application's job rather than the SDK's. `python-dotenv` is in the workspace dev group for the examples only. + +### Optional extra (`[project.optional-dependencies] otel`) + +| Package | Why | +|---|---| +| `opentelemetry-sdk>=1.25` | Tracer provider, resources, and the batch span processor, imported inside `_setup_telemetry()` in `lifecycle.py`. Optional so telemetry is opt-in; absent ⇒ a `logger.warning` and no spans, never a raise. | +| `opentelemetry-exporter-otlp-proto-http>=1.25` | OTLP/HTTP span export and its compression enum. Same optionality, same loader. | + +Install with `pip install "launchdarkly-ai-server[otel]"`; see [OTel Setup](#otel-setup) for the endpoint variables. + +### Resolved dynamically, declared nowhere + +| Package | Why | +|---|---| +| `launchdarkly-server-sdk` | The LaunchDarkly server SDK, reached by `importlib.import_module("ldclient")` (falling back to `launchdarkly_server_sdk`) inside `init_client()`'s options path. Undeclared on purpose: the BYOC path (`init_client(client=...)`) targets environments that supply their own client, and a hard dependency would force an unused SDK into every such install. So it is imported late and raises actionably when missing — absent ⇒ a `RuntimeError` naming the `pip install`, and only on the path that needs it. | + +### Dev-only (workspace root `[dependency-groups] dev`) — the ones with a contract attached + +| Package | Why | +|---|---| +| `launchdarkly-server-sdk>=9.0`, and the `otel` extra mirrored (`opentelemetry-sdk`, `opentelemetry-exporter-otlp-proto-http`) | Each dynamically-resolved or optional package is repeated in the dev group so the test suite can import it. Something that is *only* optional would not be installed in this workspace and the tests covering its present-and-working path could not run. | +| `pytest>=8`, `pytest-asyncio>=0.24` | Test runner and the async support the whole suite relies on. `asyncio_mode = "auto"` is set at the workspace root, which is why no test in this package carries an `@pytest.mark.asyncio`. | +| `mypy>=1.10` (`strict`), `ruff>=0.15` | Type checker and linter/formatter. | + +--- + ## Common Pitfalls ### 1. Calling `get_client()` before `init_client()` resolves @@ -240,6 +308,14 @@ When `enabled` is `False`, `config` is always `None`. When `enabled` is `True` b `execute_and_track` expects the handler to return a plain `dict` with at least `output` and `usage` keys. Do not return a custom class — `parse_usage` and the telemetry pipeline both access dict keys. +### 3. Interpreting skill content anywhere + +`Skill.content` is opaque `bytes` by construction. Do not add a parser, a decoder, or a +convenience accessor that reads meaning into it — no YAML/frontmatter parsing, no +"decode as UTF-8 for display", nothing. The SDK's whole contract is that content is the +verified verbatim byte buffer and nothing more; a consumer who wants structure parses it +on their side of the boundary. + --- ## Adding a New Export @@ -255,3 +331,4 @@ When `enabled` is `False`, `config` is always `None`. When `enabled` is `True` b - Handler packages must import `LDContext` from `launchdarkly-ai-server` — not directly from any LD SDK. - Do not weaken the `parse_ai_config` validation — handler packages rely on `config` being valid when they receive it. - `parse_usage` must continue to accept `input_tokens/output_tokens`, `inputTokens/outputTokens`, and `input/output` as all existing handlers return one of these variants. +- `Skill.content` is opaque `bytes`. Do not add anything that parses or interprets it — no YAML library in this package's dependencies at any tier, and no accessor that decodes content. diff --git a/packages/client/src/launchdarkly_ai_server/__init__.py b/packages/client/src/launchdarkly_ai_server/__init__.py index 80b959bc..3faff304 100644 --- a/packages/client/src/launchdarkly_ai_server/__init__.py +++ b/packages/client/src/launchdarkly_ai_server/__init__.py @@ -37,6 +37,9 @@ resolve_handlers, resolve_tools, ) +from .skills import ( + skill_refs, +) from .tracking import execute_and_stream, execute_and_track, wrap_tool_handlers from .types import ( NATIVE_TOOL_KEY, @@ -66,6 +69,8 @@ ProviderGraphResponse, ProviderHandler, ProviderResponse, + Skill, + SkillReference, StreamChunkEvent, StreamDoneEvent, StreamEvent, @@ -126,6 +131,8 @@ "ProviderGraphResponse", "ProviderHandler", "ProviderResponse", + "Skill", + "SkillReference", "StreamChunkEvent", "StreamDoneEvent", "StreamEvent", @@ -199,4 +206,6 @@ "graph", "resolve_graph", "GraphInstance", + # skills + "skill_refs", ] diff --git a/packages/client/src/launchdarkly_ai_server/skills.py b/packages/client/src/launchdarkly_ai_server/skills.py new file mode 100644 index 00000000..a3294bd2 --- /dev/null +++ b/packages/client/src/launchdarkly_ai_server/skills.py @@ -0,0 +1,72 @@ +""" +Agent Skills — reference discovery. + +Projects the skill references a resolved AI Config carries into typed values. A +pure projection: no network, no client, no store, no telemetry. Retrieving the +content those references point at is a separate layer. +""" + +from __future__ import annotations + +import logging + +from .types import AiConfigRep, SkillReference +from .types_validation import ( + is_valid_skill_key, + is_valid_skill_version, + skill_key_rejection_reason, +) + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Reference discovery +# --------------------------------------------------------------------------- +def skill_refs(config: AiConfigRep | None) -> list[SkillReference]: + """ + Projects a resolved AI Config's ``skills`` array into typed references. + + A pure projection — no network, no client, no store, no telemetry. Returns + ``[]`` when the config carries no skills. + + A config that came through ``parse_ai_config`` never contains an invalid + entry — parsing fails closed on one. A hand-built dict can, and a silently + shortened projection would leave a caller materializing a skill set it + believes is complete, so every dropped entry is logged. + """ + if not isinstance(config, dict): + return [] + + raw = config.get("skills") + if not isinstance(raw, list): + return [] + + refs: list[SkillReference] = [] + for index, entry in enumerate(raw): + if not isinstance(entry, dict): + logger.warning( + "skills[%d] is not a {key, version} object; it was dropped " + "from the projection", + index, + ) + continue + key = entry.get("key") + version = entry.get("version") + # Branch on the TypeGuard predicate (not the reason string) so the type + # checker narrows ``key`` to ``str`` for the reference below. + if not is_valid_skill_key(key): + logger.warning( + "skills[%d].key %s; it was dropped from the projection", + index, + skill_key_rejection_reason(key), + ) + elif not is_valid_skill_version(version): + logger.warning( + "skills[%d].version must be an integer >= 1; it was dropped " + "from the projection", + index, + ) + else: + refs.append(SkillReference(key=key, version=version)) + return refs diff --git a/packages/client/src/launchdarkly_ai_server/types.py b/packages/client/src/launchdarkly_ai_server/types.py index 57f81343..4249d5cb 100644 --- a/packages/client/src/launchdarkly_ai_server/types.py +++ b/packages/client/src/launchdarkly_ai_server/types.py @@ -92,7 +92,8 @@ class Message: AiConfigRep = dict[str, Any] """ Raw AI config dict as returned by ``parse_ai_config``. Fields include -``model``, ``provider``, and at least one of ``instructions`` / ``messages``. +``model``, ``provider``, at least one of ``instructions`` / ``messages``, and an +optional ``skills`` array of ``{key, version}`` references (see ``skill_refs``). """ VariationMeta = dict[str, Any] @@ -412,6 +413,45 @@ class ProviderGraphResponse: """Results from a graph-level judge, if configured.""" +# --------------------------------------------------------------------------- +# Agent Skills +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class SkillReference: + """A version-pinned pointer to a skill, as attached to an AI Config variation.""" + + key: str + """Immutable skill key — ``^[a-z0-9][a-z0-9-]*$``, at most 256 characters.""" + version: int + """Immutable skill version — an integer >= 1.""" + + +@dataclass(frozen=True) +class Skill: + """ + A single verbatim ``SKILL.md`` document. + + Only ever constructed after integrity verification passes, so ``content`` + holds the exact byte sequence LaunchDarkly delivered and ``content_hash`` + is its sha256. Instances are immutable. + """ + + key: str + version: int + content: bytes + """The verified verbatim bytes, exactly as LaunchDarkly delivered and + hashed them. Opaque to this SDK: no encoding is claimed and nothing here + ever parses or interprets them.""" + content_hash: str + """sha256, lowercase hex, over the verbatim bytes of ``content``.""" + name: str | None = None + """Display name from LaunchDarkly metadata; never parsed from the content.""" + description: str | None = None + """Description from LaunchDarkly metadata; never parsed from the content.""" + + # --------------------------------------------------------------------------- # Model / graph options # --------------------------------------------------------------------------- diff --git a/packages/client/src/launchdarkly_ai_server/types_validation.py b/packages/client/src/launchdarkly_ai_server/types_validation.py index acdcce42..37cd56bf 100644 --- a/packages/client/src/launchdarkly_ai_server/types_validation.py +++ b/packages/client/src/launchdarkly_ai_server/types_validation.py @@ -1,16 +1,62 @@ from __future__ import annotations -from typing import Any +import re +from typing import Any, TypeGuard from .types import ParseFailure, ParseResult, ParseSuccess _VALID_ROLES = {"user", "assistant", "system"} +SKILL_KEY_GRAMMAR = "^[a-z0-9][a-z0-9-]*$" +""" +The skill key grammar, as a string, so every message that has to explain a +rejection quotes the rule rather than restating it. Tightening the pattern below +then cannot leave an error message describing the old grammar. +""" + +_SKILL_KEY_PATTERN = re.compile(r"\A[a-z0-9][a-z0-9-]*\Z") +""" +``SKILL_KEY_GRAMMAR``, anchored with ``\\A``/``\\Z`` rather than ``^``/``$`` +because ``$`` also matches immediately before a trailing newline, which would +let ``"pdf-extraction\\n"`` through as a directory name. +""" + +SKILL_KEY_MAX_LENGTH = 256 +"""Longest key the data model permits. Note that no mainstream filesystem allows +a 256-byte path component, so ``write_skills`` applies a tighter bound of its own.""" + def _is_object(v: Any) -> bool: return isinstance(v, dict) +def skill_key_rejection_reason(key: Any) -> str | None: + """ + Why *key* is not a valid skill key, or ``None`` when it is. + + The canonical explanation, so the config parser, the filesystem layer and + the reference projection all reject a key for the same stated reason. + ``is_valid_skill_key`` is this predicate with the reason discarded. + """ + if not isinstance(key, str): + return "must be a string" + if len(key) > SKILL_KEY_MAX_LENGTH: + return f"must be at most {SKILL_KEY_MAX_LENGTH} characters" + if _SKILL_KEY_PATTERN.match(key) is None: + return f"must match {SKILL_KEY_GRAMMAR}" + return None + + +def is_valid_skill_key(key: Any) -> TypeGuard[str]: + """Skill keys are untrusted input everywhere they appear — validate every time.""" + return isinstance(key, str) and skill_key_rejection_reason(key) is None + + +def is_valid_skill_version(version: Any) -> TypeGuard[int]: + """Skill versions are integers >= 1. ``bool`` is not an acceptable integer.""" + return isinstance(version, int) and not isinstance(version, bool) and version >= 1 + + def _parse_tool(raw: Any, key: str) -> str | None: """Returns an error message string or ``None`` on success.""" if not _is_object(raw): @@ -24,6 +70,28 @@ def _parse_tool(raw: Any, key: str) -> str | None: return None +def _parse_skills(raw: Any) -> str | None: + """ + Validates the optional ``skills`` array. Returns an error message or ``None``. + + Fail closed: a malformed reference makes the whole config malformed, because + an SDK that silently dropped a bad reference would materialize a partial + skill set without telling anyone. + """ + if not isinstance(raw, list): + return "skills must be an array of {key, version} objects" + + for index, entry in enumerate(raw): + if not _is_object(entry): + return f"skills[{index}] must be an object with key and version" + key_rejection = skill_key_rejection_reason(entry.get("key")) + if key_rejection is not None: + return f"skills[{index}].key {key_rejection}" + if not is_valid_skill_version(entry.get("version")): + return f"skills[{index}].version must be an integer >= 1" + return None + + def parse_ai_config(raw: Any) -> ParseResult: """ Validates a raw LaunchDarkly flag variation as an ``AiConfigRep``. @@ -88,4 +156,10 @@ def parse_ai_config(raw: Any) -> ParseResult: error={"message": "outputFormat must be an object (JSON Schema)"}, ) + skills = raw.get("skills") + if skills is not None: + err = _parse_skills(skills) + if err: + return ParseFailure(success=False, error={"message": err}) + return ParseSuccess(success=True, data=raw) diff --git a/packages/client/tests/test_schema.py b/packages/client/tests/test_schema.py index fad75cf0..3be146d3 100644 --- a/packages/client/tests/test_schema.py +++ b/packages/client/tests/test_schema.py @@ -1,7 +1,8 @@ -""" -Tests for §3.5 parse_ai_config (AiConfig validation). -Reference: TESTING.md §3.5 -""" +"""Tests for ``parse_ai_config`` — AI Config variation validation.""" + +from typing import Any + +import pytest from launchdarkly_ai_server import parse_ai_config @@ -81,3 +82,97 @@ def test_output_format_accepted(self) -> None: raw["outputFormat"] = {"type": "object", "properties": {}} result = parse_ai_config(raw) assert result.success is True + + +class TestParseAiConfigSkills: + """ + Fail-closed validation of the optional ``skills`` array. + """ + + def _base(self, **extra: Any) -> dict[str, Any]: + raw: dict[str, Any] = { + "model": {"name": "claude-3"}, + "provider": {"name": "Anthropic"}, + "instructions": "You are helpful.", + } + raw.update(extra) + return raw + + def test_absent_skills_is_valid(self) -> None: + assert parse_ai_config(self._base()).success is True + + def test_empty_skills_is_valid(self) -> None: + assert parse_ai_config(self._base(skills=[])).success is True + + def test_valid_entries_accepted(self) -> None: + raw = self._base(skills=[{"key": "pdf-extraction", "version": 2}]) + result = parse_ai_config(raw) + assert result.success is True + assert result.data["skills"] == [{"key": "pdf-extraction", "version": 2}] + + def test_multiple_valid_entries_accepted(self) -> None: + raw = self._base( + skills=[{"key": "a", "version": 1}, {"key": "b-2", "version": 10}] + ) + assert parse_ai_config(raw).success is True + + def test_key_at_length_bound_accepted(self) -> None: + raw = self._base(skills=[{"key": "a" * 256, "version": 1}]) + assert parse_ai_config(raw).success is True + + @pytest.mark.parametrize("bad_skills", ["pdf", {"key": "a"}, 3, True]) + def test_non_array_skills_fails(self, bad_skills: Any) -> None: + assert parse_ai_config(self._base(skills=bad_skills)).success is False + + @pytest.mark.parametrize("entry", ["pdf-extraction", 1, None, ["a", 1]]) + def test_non_object_entry_fails(self, entry: Any) -> None: + assert parse_ai_config(self._base(skills=[entry])).success is False + + @pytest.mark.parametrize("bad_key", [None, 1, True, {"a": 1}, ["a"]]) + def test_missing_or_non_string_key_fails(self, bad_key: Any) -> None: + raw = self._base(skills=[{"key": bad_key, "version": 1}]) + assert parse_ai_config(raw).success is False + + def test_absent_key_fails(self) -> None: + assert parse_ai_config(self._base(skills=[{"version": 1}])).success is False + + @pytest.mark.parametrize( + "bad_key", + [ + "", + "Evil", + "-leading-dash", + ".hidden", + "_underscore", + "has space", + "a/b", + "a\\b", + "../escape", + "trailing-space ", + "under_score", + "a" * 257, + ], + ) + def test_pattern_and_length_violations_fail(self, bad_key: str) -> None: + raw = self._base(skills=[{"key": bad_key, "version": 1}]) + assert parse_ai_config(raw).success is False + + @pytest.mark.parametrize("bad_version", [0, -1, 2.5, "2", None, True, [1]]) + def test_invalid_version_fails(self, bad_version: Any) -> None: + raw = self._base(skills=[{"key": "a", "version": bad_version}]) + assert parse_ai_config(raw).success is False + + def test_absent_version_fails(self) -> None: + assert parse_ai_config(self._base(skills=[{"key": "a"}])).success is False + + def test_one_bad_entry_fails_the_whole_config(self) -> None: + raw = self._base( + skills=[{"key": "good", "version": 1}, {"key": "../bad", "version": 1}] + ) + assert parse_ai_config(raw).success is False + + def test_error_message_mentions_skills(self) -> None: + raw = self._base(skills=[{"key": "../bad", "version": 1}]) + result = parse_ai_config(raw) + assert result.success is False + assert "skills" in result.error["message"] diff --git a/packages/client/tests/test_skills.py b/packages/client/tests/test_skills.py new file mode 100644 index 00000000..38ca34f2 --- /dev/null +++ b/packages/client/tests/test_skills.py @@ -0,0 +1,139 @@ +""" +Agent Skills — reference discovery: the types and the projection from a +resolved AI Config. +""" + +from __future__ import annotations + +import dataclasses +import hashlib +from typing import Any + +import pytest + +from launchdarkly_ai_server import ( + Skill, + SkillReference, + skill_refs, +) + +SKILL_BODY = "---\nname: Test Skill\n---\nDo the thing.\n" + + +def _hash(content: str) -> str: + """sha256, lowercase hex, over verbatim utf-8 bytes.""" + return hashlib.sha256(content.encode("utf-8")).hexdigest() + + +def _skill( + key: str = "test-skill", + version: int = 1, + content: str = SKILL_BODY, + content_hash: str | None = None, +) -> Skill: + """Build a verified-shaped Skill directly (bypasses the accessors).""" + return Skill( + key=key, + version=version, + content=content.encode("utf-8"), + content_hash=content_hash if content_hash is not None else _hash(content), + ) + + +class TestSkillTypes: + """Immutability and optional metadata.""" + + def test_skill_reference_is_immutable(self) -> None: + ref = SkillReference(key="pdf-extraction", version=2) + with pytest.raises(dataclasses.FrozenInstanceError): + ref.version = 3 # type: ignore[misc] + + def test_skill_is_immutable(self) -> None: + skill = _skill() + with pytest.raises(dataclasses.FrozenInstanceError): + skill.content = b"tampered" # type: ignore[misc] + + def test_skill_content_is_bytes(self) -> None: + """Content is the verified verbatim bytes — opaque, never text.""" + skill = _skill() + assert isinstance(skill.content, bytes) + assert skill.content == SKILL_BODY.encode("utf-8") + + def test_skill_carries_optional_metadata(self) -> None: + skill = Skill( + key="a", + version=1, + content=SKILL_BODY.encode("utf-8"), + content_hash=_hash(SKILL_BODY), + name="A Skill", + description="does things", + ) + assert skill.name == "A Skill" + assert skill.description == "does things" + + def test_skill_metadata_defaults_to_none(self) -> None: + skill = _skill() + assert skill.name is None + assert skill.description is None + + +class TestSkillRefs: + """Pure projection of the config's skills array.""" + + def _config(self, **extra: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "model": {"name": "claude-3"}, + "provider": {"name": "Anthropic"}, + "instructions": "hi", + } + base.update(extra) + return base + + def test_absent_skills_returns_empty_list(self) -> None: + assert skill_refs(self._config()) == [] + + def test_empty_skills_returns_empty_list(self) -> None: + assert skill_refs(self._config(skills=[])) == [] + + def test_returns_typed_references_in_order(self) -> None: + config = self._config( + skills=[{"key": "a", "version": 1}, {"key": "b", "version": 3}] + ) + refs = skill_refs(config) + assert refs == [ + SkillReference(key="a", version=1), + SkillReference(key="b", version=3), + ] + assert all(isinstance(r, SkillReference) for r in refs) + + def test_dropped_entries_are_logged(self, caplog: pytest.LogCaptureFixture) -> None: + """A shortened projection is never silent. + + ``parse_ai_config`` fails the whole config closed on a malformed + reference, so a config that reached here through it cannot contain one. + A hand-built dict can, and feeding the shortened list to + ``write_skills`` would prune the dropped skill's on-disk copy — so the + drop is observable rather than silent. + """ + config = self._config( + skills=[ + {"key": "good", "version": 1}, + {"key": "bad", "version": 0}, + {"key": "Bad-Key", "version": 1}, + "not-an-object", + ] + ) + + with caplog.at_level("WARNING", logger="launchdarkly_ai_server.skills"): + refs = skill_refs(config) + + assert refs == [SkillReference(key="good", version=1)] + assert len(caplog.records) == 3 + # The body is never echoed, and neither is the invalid key. + assert all("skills[" in r.getMessage() for r in caplog.records) + + def test_requires_no_client_or_store(self, mock_ld_client: Any) -> None: + """No store configured, no client initialized — still a pure projection.""" + refs = skill_refs(self._config(skills=[{"key": "a", "version": 2}])) + assert refs == [SkillReference(key="a", version=2)] + mock_ld_client.track.assert_not_called() diff --git a/uv.lock b/uv.lock index de575a71..7d93a3cd 100644 --- a/uv.lock +++ b/uv.lock @@ -790,7 +790,7 @@ wheels = [ [[package]] name = "launchdarkly-ai-claude-agents" -version = "0.1.1" +version = "0.1.4" source = { editable = "packages/claude-agents" } dependencies = [ { name = "anthropic" }, @@ -809,7 +809,7 @@ requires-dist = [ [[package]] name = "launchdarkly-ai-claude-messages" -version = "0.1.1" +version = "0.1.4" source = { editable = "packages/claude-messages" } dependencies = [ { name = "anthropic" }, @@ -826,7 +826,7 @@ requires-dist = [ [[package]] name = "launchdarkly-ai-langchain-agents" -version = "0.1.1" +version = "0.1.4" source = { editable = "packages/langchain-agents" } dependencies = [ { name = "langchain-core" }, @@ -845,7 +845,7 @@ requires-dist = [ [[package]] name = "launchdarkly-ai-langchain-messages" -version = "0.1.1" +version = "0.1.4" source = { editable = "packages/langchain-messages" } dependencies = [ { name = "langchain-core" }, @@ -862,7 +862,7 @@ requires-dist = [ [[package]] name = "launchdarkly-ai-openai-agents" -version = "0.1.1" +version = "0.1.4" source = { editable = "packages/openai-agents" } dependencies = [ { name = "launchdarkly-ai-server" }, @@ -881,7 +881,7 @@ requires-dist = [ [[package]] name = "launchdarkly-ai-openai-messages" -version = "0.1.1" +version = "0.1.4" source = { editable = "packages/openai-messages" } dependencies = [ { name = "launchdarkly-ai-server" }, @@ -898,7 +898,7 @@ requires-dist = [ [[package]] name = "launchdarkly-ai-python" -version = "0.1.1" +version = "0.1.3" source = { editable = "packages/ai" } dependencies = [ { name = "launchdarkly-ai-server" }, @@ -918,7 +918,7 @@ provides-extras = ["otel"] [[package]] name = "launchdarkly-ai-server" -version = "0.1.1" +version = "0.1.3" source = { editable = "packages/client" } dependencies = [ { name = "opentelemetry-api" },