feat(client): Agent Skills — re-reconcile on delivery with watch_skills

packages/client/README.md

@@ -445,6 +445,66 @@ which OS ran the write. The keys stay valid everywhere else: an AI Config refere
 named `aux` parses, and its other fields are unaffected. If you have a skill named for a
 device, rename it.
 
+#### Receiving skills from LaunchDarkly
+
+`InMemorySkillStore` is for tests and bring-your-own-content. In production, skill content
+arrives through `FDv2SkillStore`, which speaks LaunchDarkly's SDK-facing FDv2 delivery
+channel — the same `GET /sdk/poll` and `GET /sdk/stream` endpoints the base SDK's FDv2 data
+source uses, authenticated with the environment's server-side SDK key.
+
+```python
+import os
+
+from launchdarkly_ai_server import FDv2SkillStore, init_client, watch_skills
+
+store = FDv2SkillStore(os.environ["LD_SDK_KEY"]).start()
+store.wait_for_skills(timeout=10)
+await init_client(options={"skillStore": store})
+
+# Materialize now, and re-materialize whenever delivery changes.
+report, watcher = await watch_skills("*", ".claude/skills")
+try:
+    ...
+finally:
+    watcher.close()
+    store.close()
+```
+
+**Nothing above the store changes.** The accessors, verification, and `write_skills` see raw
+objects through the `SkillStore` interface and cannot tell which store produced them.
+
+**Server-side only.** Skills are for server-side agent runtimes and skill content is
+customer-confidential. A mobile key (`mob-…`) or a client-side environment ID raises from the
+constructor.
+
+**Streaming is the default, and it is what makes revocation fast.** A `delete-object` reaches
+a live stream in seconds; with `mode="poll"` it arrives within one `poll_interval`. Paired
+with `watch_skills`, a revoked skill's `SKILL.md` leaves the disk without a restart. During an
+outage the store keeps serving the last content it received and `write_skills`' default
+`on_unavailable="keep"` leaves managed files alone — an outage must not read as "everything
+was revoked".
+
+**One network timeout, and its default depends on the mode.** `read_timeout` bounds every
+socket operation of a request, connecting included. In `mode="poll"` it bounds the whole
+request and defaults to 10 seconds; in `mode="stream"` it bounds each wait for the next bytes
+and defaults to 300 seconds, well beyond LaunchDarkly's heartbeat interval.
+
+**The connection also carries your flags.** A client cannot request only the skill payload,
+so a skills-enabled environment delivers flag and segment objects on the same connection.
+They are skipped, not evaluated — this store does no evaluation of any kind — and
+`diagnostics.objects_ignored` counts them.
+
+> **Beta caveats, worth knowing before you deploy.** Payload signing does not exist on this
+> channel yet, so delivery is TLS-only and the content hash establishes self-consistency, not
+> origin authenticity. The FDv2 protocol is opt-in per account: without it the endpoints
+> return HTTP 403, which the store reports as a fatal error explaining what to do. `ld-relay`
+> does not speak the FDv2 endpoints, so relay-only deployments cannot receive skills.
+
+**If every skill comes back empty, check `diagnostics.hashless_objects`.** Verification
+withholds any delivered object without a `contentHash`, so a nonzero count means skills are
+being withheld rather than that the environment has none. The store also logs an error per
+hashless object naming the reason. There is deliberately no fallback that skips verification.
+
 **Total path length is yours to bound, not the SDK's.** The 255-byte bound above is per
 *component*; the root is your path, so `<root>` + `<key>` + `/SKILL.md` can still exceed
 Windows' 260-character `MAX_PATH` with a perfectly legal key. Choose a short managed root on
@@ -458,8 +518,11 @@ Windows.
 | `get_skills(refs)` | Batch form. Accepts `SkillReference` values and bare key strings (string = latest). Results follow input order; missing or unverifiable entries are omitted. |
 | `all_skills()` | Every verified skill the store holds, one per key at its newest version. |
 | `write_skills(skills, root, *, prune=True, timeout=10.0, on_unavailable="keep")` | Materialize skills under `root`, returning a `ReconcileReport`. `prune` removes formerly-managed skills no longer requested. `on_unavailable="raise"` raises instead of reporting when content cannot be retrieved. Raises `ValueError` for an unusable root, a negative `timeout`, or an unrecognised `on_unavailable`. **Performs synchronous filesystem I/O — see the note below.** |
-| `SkillStore` | The structural interface content arrives through: `get_object(kind, key, version=None)`, `all_objects(kind)`, optional `add_listener(kind, fn)`. |
+| `SkillStore` | The structural interface content arrives through: `get_object(kind, key, version=None)`, `all_objects(kind)`, optional `add_listener(kind, fn)` / `remove_listener(kind, fn)`. |
 | `InMemorySkillStore(objects=None)` | A dict-backed store with `put(raw)`, for local development and testing. Holds several versions of a key. |
+| `FDv2SkillStore(sdk_key, *, base_uri=…, mode="stream", …)` | The delivery transport: a store fed by LaunchDarkly over the SDK-facing FDv2 channel. `start()`, `wait_for_skills(timeout)`, `close()`, `diagnostics`, `failed`; also a context manager. **Server-side only** — a mobile key or client-side environment ID raises. See *Receiving skills from LaunchDarkly* above. |
+| `watch_skills(skills, root, …)` | `write_skills` plus a re-reconcile on every delivery change. Returns `(initial report, SkillWatcher)`; close the watcher when done. Revocation then takes effect within `debounce` of arriving rather than at the next restart. |
+| `StoreDiagnostics` | What the transport has seen: `payloads_transferred`, `skill_objects_received`, `objects_ignored`, `objects_revoked`, `hashless_objects`, `connection_failures`, `last_error`. |
 
 Configure the store with `init_client(options={"skillStore": store})`. With none configured,
 the accessors raise `RuntimeError` explaining what to do and `write_skills` reports the

packages/client/agents.md

@@ -30,6 +30,8 @@ No other `launchdarkly-ai-*` package may define or duplicate these. They import
 | `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, retrieval half — `skill_refs`, `get_skill`/`get_skills`/`all_skills`, `InMemorySkillStore`, and the store/telemetry injection points `_set_store` / `_set_emitter_for_testing` |
 | `src/launchdarkly_ai_server/skills_core.py` | Shared skills internals — the `SkillStore` seam, module state, the telemetry seam and its three recorders, integrity verification, and store resolution. Imported by both `skills.py` and the materialization layer; imports neither |
+| `src/launchdarkly_ai_server/skills_fdv2.py` | Agent Skills, delivery transport — the FDv2 protocol, the wire-key/`version` translation, the held object set, and `FDv2SkillStore`. Sits **below** the store interface; imports `skills_core` only, and nothing imports it |
+| `src/launchdarkly_ai_server/skills_watch.py` | Agent Skills, eager re-reconcile — `watch_skills` / `SkillWatcher`, wiring the store's change listener to `write_skills`. Sits **above** `skills_fs` and modifies none of it |
 | `src/launchdarkly_ai_server/skills_fs.py` | Agent Skills, materialization half — `write_skills`, request resolution, the manifest format and on-disk filenames, per-skill reconcile, and pruning |
 | `src/launchdarkly_ai_server/safe_fs.py` | Descriptor-pinned filesystem primitives — `atomic_write`, `unlink_file`, `pinned_directory`, `open_directory_nofollow`, `open_or_create_directory`, `SymlinkRefused`, and the `*at()` capability probe. Owns the descriptor-vs-path platform split; knows nothing about skills |
 | `src/launchdarkly_ai_server/utils.py` | `parse_template`, `parse_json_with_possible_fences`, `create_handler`, `parse_usage`, `make_track_data`, `to_ld_context` |
@@ -198,11 +200,11 @@ Three layers, in increasing order of blast radius:
 
 ### The store seam, and why version is part of the lookup
 
-`SkillStore` is `get_object(kind, key, version=None)`, `all_objects(kind)`, and an optional
-`add_listener(kind, fn)`. Version is part of the **lookup identity**, not a filter applied
-to the answer, and that is load-bearing: a delivery payload carries the newest version of
-every skill *plus* every version any variation currently pins, so two versions of one key
-coexist routinely. A seam keyed by key alone would answer a pinned reference with the newest
+`SkillStore` is `get_object(kind, key, version=None)`, `all_objects(kind)`, and the optional
+pair `add_listener(kind, fn)` / `remove_listener(kind, fn)`. Version is part of the **lookup
+identity**, not a filter applied to the answer, and that is load-bearing: a delivery payload
+carries the newest version of every skill *plus* every version any variation currently pins,
+so two versions of one key coexist routinely. A store keyed by key alone would answer a pinned reference with the newest
 object, and the caller would then have to reject it — turning the primary use case, a
 version-pinned attachment, into a missing skill. `version=None` asks for the newest held.
 
@@ -217,6 +219,87 @@ one place that collapses the result to one object per key, because both whole-st
 consumers need it — `all_skills`, since a list holding two versions of one key is not a set
 of skills, and the `"*"` reconcile, since `<root>/<key>/SKILL.md` is a single path.
 
+### The delivery transport, and the one field that will bite you
+
+`FDv2SkillStore` speaks LaunchDarkly's SDK-facing FDv2 channel (`GET /sdk/poll`,
+`GET /sdk/stream`, server-side SDK key in `Authorization`, `basis` + `mv` params,
+`If-None-Match`/304). It lives below the store interface and produces raw objects in the
+shape `skills_core.SkillStore` documents; **nothing above that interface knows it exists**. If a transport
+change ever seems to require editing an accessor, verification, or `write_skills`, the adapter
+boundary is wrong.
+
+**The skill's version is in the object's `key`. `version` is the payload's.** Each version
+of a skill is its own object on the wire, identified as `<key>:<version>`:
+
+```json
+{"key":"pdf-extraction:3","kind":"skill","version":42,
+ "object":{"contentType":"text/markdown","content":"…","contentHash":"…","name":"…"}}
+```
+
+The `3` after the delimiter is what a `{key, version}` reference pins and what becomes the
+stored `version`, under the stored key `pdf-extraction`. `version` (42) is the version of the
+*payload* the object arrived in — it moves when anything in the environment moves,
+including a flag with nothing to do with skills. Reading it as the skill's version fails
+**silently**: the object verifies, the hash matches, and the caller gets content under a
+version number that means nothing. There is no separate field for the skill's version: the
+agent-skill payload is a *generic* payload, and generic objects carry only `key`, `kind`,
+`version` and `object`, exactly like a flag. `_split_wire_key` is the only place the wire key
+is read, `_store_object_from_put` and `_tombstone_from_delete` both go through it, and
+`TestVersionTranslation` asserts the translation in both directions. A wire key that will
+not split cleanly is *held*, not dropped — version-less, or with the offending text as its
+version — so verification withholds it with `invalid_version` under a key the caller
+recognises; only a key with nothing before the delimiter is dropped, since there is no
+identity to hold it under.
+
+**Skills are identified by `kind == "skill"`; everything else is ignored, not rejected.**
+Object kinds on the SDK-facing channel are open strings, and the agent-skill payload is
+classified `generic`, so a skill arrives under the kind its producer registered — the bare
+category name — not under a broader wrapper kind with a narrowing field. An environment's
+payload assignment carries its flag payload alongside its agent-skill payload, so flag and
+segment objects arrive as a matter of course. Erroring on an unrecognised kind would turn a
+normal payload into a permanent reconnect loop — a flag-delivery outage caused by a skills
+rollout.
+
+**Changes commit at `payload-transferred`, not as objects arrive.** A payload version is the
+unit of consistency: a half-applied full transfer would publish a state the server never
+described, and would briefly empty the store — which, with pruning on, is the difference
+between a reconcile and deleting a customer's skill files. An interrupted transfer therefore
+leaves last known good intact, and listeners fire once per commit.
+
+**The first payload intent is read, and is assumed to be the skill payload.** Delivery
+provides one payload per credential and the protocol requires a client to ignore all but the
+first payload intent, so `payloads[0]` is both what arrives and what the protocol says to
+read. The cost of that assumption is that an `xfer-full` for somebody *else's* payload would
+start an empty pending set, and the next `payload-transferred` would publish it — every skill
+reported revoked, and with pruning on, a customer's files deleted. `_ProtocolReader`
+therefore learns which payload skills arrive on, from the intent's `id` or from the
+`(p:<id>:<version>)` selector, and declines to apply a transfer of any other: once at
+WARNING, counted in `diagnostics.payloads_ignored`, holding last known good. A transfer that
+names no payload is applied, since one-payload delivery is the common case. The residual is
+the first transfer of a connection — before a skill has arrived there is nothing to compare
+against — which is what the separate WARNING on a multi-payload intent is for.
+
+**A hashless object is held, not dropped.** Verification withholds it with
+`missing_content_hash`; the transport's job is to make that loud (an error per object, a
+summary per wholly-hashless payload, `diagnostics.hashless_objects`) rather than to work
+around it. Dropping it at the transport would report `absent` — indistinguishable from "no
+such skill" — and would let a prune delete the last known-good copy on disk. Never synthesize
+a hash from the delivered content: that certifies the content against itself and verifies
+nothing.
+
+**There is one network timeout, not two.** `urllib`'s `timeout` is the socket timeout for the
+whole operation, so connect, headers and each read share it, and the module cannot bound the
+connect separately without a custom connection class it should not carry. `read_timeout` is
+therefore the only knob, and its default is per mode (`DEFAULT_POLL_TIMEOUT` for a whole poll
+request, `DEFAULT_STREAM_READ_TIMEOUT` for the gap between reads on a stream). Do not add a
+parameter that the standard library cannot honour; `TestTimeouts` measures the bound against a
+socket that accepts and never answers.
+
+**`close` interrupts the socket, it does not just set a flag.** The delivery thread spends its
+life blocked in a read that no flag can reach, and closing a response from another thread does
+not unblock CPython's buffered reader. `_interrupt_read` shuts the socket down underneath it.
+Without that, every shutdown of a *healthy* stream blocks for the full join timeout.
+
 ### The reported outcome vocabulary, and the `Resolution` mapping
 
 `get_skill` returns `Skill | None`; `get_skill_result` returns a frozen `SkillOutcome`

packages/client/src/launchdarkly_ai_server/__init__.py

@@ -46,13 +46,15 @@
     skill_refs,
 )
 from .skills_core import SkillStore
+from .skills_fdv2 import FDv2SkillStore, StoreDiagnostics
 from .skills_fs import (
     MANIFEST_FILENAME,
     MANIFEST_VERSION,
     SKILL_FILENAME,
     OnUnavailable,
     write_skills,
 )
+from .skills_watch import SkillWatcher, watch_skills
 from .tracking import execute_and_stream, execute_and_track, wrap_tool_handlers
 from .types import (
     NATIVE_TOOL_KEY,
@@ -237,6 +239,11 @@
     "write_skills",
     "SkillStore",
     "InMemorySkillStore",
+    # skills — the FDv2 delivery transport, and the eager re-reconcile it enables
+    "FDv2SkillStore",
+    "StoreDiagnostics",
+    "watch_skills",
+    "SkillWatcher",
     # skills — the three closed-set unions a typed consumer needs to name
     "ReconcileActionKind",
     "OnUnavailable",

packages/client/src/launchdarkly_ai_server/skills.py

@@ -167,6 +167,22 @@ def add_listener(self, kind: str, fn: Callable[[dict[str, Any]], Any]) -> None:
         """
         self._listeners.setdefault(kind, []).append(fn)
 
+    def remove_listener(self, kind: str, fn: Callable[[dict[str, Any]], Any]) -> None:
+        """
+        Unregisters *fn* from *kind*, so a subsequent ``put`` no longer calls it.
+
+        Removes one occurrence: a callable registered twice must be removed twice.
+        Removing a callable that is not registered is a no-op, not an error, so a
+        consumer that detaches on close can do so unconditionally.
+        """
+        listeners = self._listeners.get(kind)
+        if listeners is None:
+            return
+        try:
+            listeners.remove(fn)
+        except ValueError:
+            return
+
 
 # ---------------------------------------------------------------------------
 # Reference discovery