feat(client): declare ?kinds=agent-skill, and treat a 422 as "no skills yet"

packages/client/README.md

@@ -542,6 +542,14 @@ that as every skill having been revoked and delete the files it wrote on a previ
 against a store that has not received a payload reports the retrieval unavailable and leaves
 everything on disk alone. `report.ok` is `False` in that case, and the error names it.
 
+**A project with no skills yet is a waiting state, not a failure.** Every request declares
+the payload it wants (`kinds=agent-skill`), and LaunchDarkly answers HTTP 422 when the
+environment has no such payload — which is the case until somebody creates the first skill in
+the project. The store says so once, keeps asking at its backoff cap, and picks up that first
+skill without a restart. `failed` stays `None` throughout, `connection_failures` stays at
+zero, and `diagnostics.payload_unavailable` counts the answers: nonzero and rising beside an
+empty store means "there is nothing to deliver", not "delivery is broken".
+
 **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.
 
@@ -622,7 +630,7 @@ Windows.
 | `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=…, stream_uri=…, mode="stream", …)` | The delivery transport: a store fed by LaunchDarkly over the SDK-facing FDv2 channel. `start()`, `wait_for_skills(timeout)`, `is_initialized()`, `close()`, `diagnostics`, `failed`; also a context manager. `base_uri` and `stream_uri` are separate hosts, defaulting to LaunchDarkly's polling and streaming origins; `base_uri` alone covers both. `close()` is **final** — `start()` afterwards raises. **Server-side only** — a mobile key or client-side environment ID raises. See *Receiving skills from LaunchDarkly* above. |
 | `watch_skills(skills, root, *, debounce=0.5, on_reconcile=None, …)` | `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. `debounce` is in **seconds** and must be non-negative and finite; `on_reconcile` is called with each *subsequent* report, the initial one being returned directly. One watcher per root. |
-| `StoreDiagnostics` | What the transport has seen: `payloads_transferred`, `skill_objects_received`, `objects_ignored`, `objects_revoked`, `hashless_objects`, `connection_failures`, `last_error`. |
+| `StoreDiagnostics` | What the transport has seen: `payloads_transferred`, `skill_objects_received`, `objects_ignored`, `objects_revoked`, `hashless_objects`, `connection_failures`, `payload_unavailable`, `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

@@ -250,12 +250,36 @@ deletes the user's files.
 ### 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,
+`GET /sdk/stream`, server-side SDK key in `Authorization`, `kinds` + `basis` 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.
 
+**Every request declares `kinds=agent-skill`, and that is load-bearing.** Delivery narrows a
+connection to the payload kinds it declares and defaults to flags, so a request without it is
+served the environment's flag payload and no skills at all — the store would run, report
+healthy, and hold nothing. It also makes the connection carry exactly one payload, which is
+the shape `_ProtocolReader` is built for: without it, a skill-enabled environment assigns two,
+and the reader warns about the second and reads only the first intent. `FDV2_PAYLOAD_KIND` is
+the payload's kind and `FDV2_OBJECT_KIND` the kind of the objects inside it — two different
+strings, held apart on purpose. No `mv`: that parameter selects the *flag* data model, and
+delivery overrides whatever a request asks for with the payload's own default for any
+non-flagging payload, so sending it would state a preference that is ignored.
+
+**HTTP 422 is not a failure.** It is the answer to that declaration when the credential is
+assigned no agent-skill payload, which is every project in which no skill has ever been
+created — gonfalon creates that payload row with the first skill and never lazily. So
+`_classify_status` maps it to `_NoSkillPayloadError`, and `_run` catches that **ahead of**
+`_RecoverableTransportError`: counted under `diagnostics.payload_unavailable`, logged once per
+store, retried at `max_backoff` indefinitely, and kept off `connection_failures`,
+`last_error`, and `failed`. Neither of the two obvious classifications is right — as a
+recoverable failure it spends `max_consecutive_failures` and then gives up permanently on an
+ordinary configuration; as a fatal one, the skill created a minute later never arrives without
+a process restart. The retry is at the *cap* rather than the initial backoff because
+`_failures` deliberately never moves, so the exponential schedule would sit at the initial
+delay forever.
+
 **The key travels over TLS only, and only to the base URI.** `_require_https_base_uri` refuses a
 plain `http://` base URI in the constructor — the SDK key would go out in cleartext — with a
 loopback exemption (`localhost`, `127.0.0.1`, `::1`) because the test suite's fake endpoints
@@ -287,7 +311,7 @@ stored `version`, under the stored key `pdf-extraction`. `version` (42) is the v
 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`,
+agent-skill payload is opaque to delivery, and its 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
@@ -297,13 +321,13 @@ recognises; only a key with nothing before the delimiter is dropped, since there
 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.
+Object kinds on the SDK-facing channel are open strings, so a skill arrives under the kind its
+producer registered — the bare category name — not under a broader wrapper kind with a
+narrowing field. The `kinds` declaration above means a flag or segment object should no longer
+arrive at all, but the skip stays and stays tested: erroring on an unrecognised kind would
+turn a payload that merely gained a new object kind into a permanent reconnect loop — a
+flag-delivery outage caused by a skills rollout — and an object nobody expected belongs
+outside the skill set rather than in it.
 
 **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

packages/client/src/launchdarkly_ai_server/skills_fdv2.py

@@ -66,12 +66,31 @@
 """
 The FDv2 ``kind`` skills are delivered under.
 
-Object kinds on the SDK-facing channel are open strings: the agent-skill payload
-is classified ``generic`` and every object in it carries the kind its producer
-registered, which for skills is the bare category name. Delivery lower-cases the
-kind, so an exact comparison is the whole test. The value happens to equal
-``skills_core.SKILL_OBJECT_KIND``; they remain separate constants, because one is
-a wire value LaunchDarkly owns and the other is what this SDK asks a store for.
+Object kinds on the SDK-facing channel are open strings: every object in the
+agent-skill payload carries the kind its producer registered, which for skills is
+the bare category name. Delivery lower-cases the kind, so an exact comparison is
+the whole test. The value happens to equal ``skills_core.SKILL_OBJECT_KIND``;
+they remain separate constants, because one is a wire value LaunchDarkly owns and
+the other is what this SDK asks a store for.
+
+Not to be confused with ``FDV2_PAYLOAD_KIND``: this is the kind of the *objects*,
+that one the kind of the *payload* they arrive in.
+"""
+
+FDV2_PAYLOAD_KIND = "agent-skill"
+"""
+The kind of the FDv2 payload skills are delivered in, declared on every request
+as ``?kinds=``.
+
+Delivery narrows a connection to the payload kinds it declares and defaults to
+flags, so this is not an optimisation: a request that omits it receives the
+environment's flag payload and no skills at all. Declaring it is also what makes
+the connection carry exactly one payload -- the shape ``_ProtocolReader`` is
+built for -- since a skill-enabled environment assigns both the flag payload and
+this one.
+
+The wire accepts a comma-separated list, but this store wants the skill payload
+and nothing else, so it declares this one kind alone.
 """
 
 FDV2_KEY_DELIMITER = ":"
@@ -287,6 +306,16 @@ class StoreDiagnostics:
     """
     connection_failures: int = 0
     """Recoverable transport failures since the last successful transfer."""
+    payload_unavailable: int = 0
+    """
+    Requests answered with "no payload of the kind you asked for"
+    (``_NoSkillPayloadError``). Cumulative, and never reset.
+
+    Deliberately not a ``connection_failures``: nothing is wrong, there is
+    nothing to deliver. Nonzero and rising alongside an empty store is the
+    difference between "this environment has no skills" and "delivery is
+    broken", which is the pair this whole type exists to separate.
+    """
     last_error: str | None = None
     """The most recent transport error, if any. Human-readable; do not parse."""
 
@@ -973,6 +1002,26 @@ def __init__(self, message: str, retry_after: float | None = None) -> None:
         self.retry_after = retry_after
 
 
+class _NoSkillPayloadError(_RecoverableTransportError):
+    """
+    An HTTP 422: delivery has no payload of the kind this store declared.
+
+    That is the answer for every project in which no skill has ever been
+    created, since the agent-skill payload row is created with the first one.
+    Neither of the two obvious classifications is right, which is why this is
+    its own class:
+
+    - as a failure it would spend ``max_consecutive_failures`` and then give up
+      permanently -- "gave up after N consecutive failures" -- on a
+      configuration that is merely waiting for its first skill;
+    - as fatal, the skill created a minute later would never arrive, because
+      nothing reopens delivery short of a process restart.
+
+    ``_run`` handles it ahead of ``_RecoverableTransportError``, which it
+    subclasses.
+    """
+
+
 class _StaleRequestStateError(_RecoverableTransportError):
     """
     An HTTP 400 for a request carrying client state — the ``basis`` selector, or
@@ -1057,6 +1106,14 @@ def _classify_status(status: int, headers: Any) -> Exception:
         return _StaleRequestStateError(
             f"LaunchDarkly returned HTTP 400. {_REQUEST_ADVICE}"
         )
+    if status == 422:
+        return _NoSkillPayloadError(
+            "LaunchDarkly has no Agent Skills payload for this environment "
+            "(HTTP 422). This is what it answers until the first skill is "
+            "created in this project, so delivery keeps asking and picks one up "
+            "without a restart. If this environment does have skills, check that "
+            "this SDK key belongs to it."
+        )
     if status in (405, 406, 414, 501):
         return _FatalTransportError(
             f"LaunchDarkly returned HTTP {status}, which retrying will not fix. "
@@ -1198,19 +1255,25 @@ def interrupt(self) -> None:
 
     def _url(self, origin: str, path: str, basis: str | None) -> str:
         """
-        The request URL: the path, plus ``basis`` once a payload has committed.
+        The request URL: the path, the payload kind this store accepts, and
+        ``basis`` once a payload has committed.
 
         *origin* is the host for this path — polling and streaming have one
         each.
 
+        ``kinds`` is on every request, including the first one, because it
+        selects what the connection is served rather than describing what it
+        already holds (see ``FDV2_PAYLOAD_KIND``).
+
         Deliberately no ``mv`` (data model version). That parameter selects the
-        *flag* data model and the connection rejects any value but the flag
-        default; the agent-skill payload is generic, is served regardless of it,
-        and has no model version of its own to ask for.
+        *flag* data model; delivery overrides whatever a request asks for with
+        the payload's own default for any non-flagging payload, so sending it
+        would state a preference that is ignored.
         """
-        if not basis:
-            return f"{origin}{path}"
-        return f"{origin}{path}?{urllib.parse.urlencode({'basis': basis})}"
+        query: dict[str, str] = {"kinds": FDV2_PAYLOAD_KIND}
+        if basis:
+            query["basis"] = basis
+        return f"{origin}{path}?{urllib.parse.urlencode(query)}"
 
     def _request(
         self, origin: str, path: str, basis: str | None, headers: dict[str, str]
@@ -1633,6 +1696,9 @@ def __init__(
         # A stream only ever ends by being dropped, so this is what separates
         # a recycled healthy connection from one that failed.
         self._attempt_answered = False
+        # Said once per store rather than once per attempt: the condition holds
+        # until somebody creates a skill, and delivery keeps asking throughout.
+        self._warned_no_skill_payload = False
 
     # -- lifecycle ---------------------------------------------------------
 
@@ -1890,6 +1956,32 @@ def _run(self) -> None:
             except _FatalTransportError as exc:
                 self._give_up(str(exc))
                 return
+            except _NoSkillPayloadError as exc:
+                # Ahead of the recoverable block below, none of which applies:
+                # nothing failed, so there is no count to advance and no
+                # ``last_error`` to leave on a store that is working fine.
+                if self._stop.is_set():
+                    return
+                with self._lock:
+                    # A 422 refuses the connection before it opens, so this is
+                    # only for a payload an earlier attempt left in flight.
+                    self._reader._abandon_in_flight()
+                    self._reader.diagnostics.payload_unavailable += 1
+                    say_it = not self._warned_no_skill_payload
+                    self._warned_no_skill_payload = True
+                if say_it:
+                    logger.warning(
+                        "Skill delivery is idle: %s Retrying every %.1fs; this "
+                        "is the only time it will be said.",
+                        exc,
+                        self._max_backoff,
+                    )
+                # At the cap rather than on the backoff schedule: ``_failures``
+                # deliberately never moves, so the schedule would hold this at
+                # the *initial* delay forever.
+                if self._stop.wait(self._max_backoff):
+                    return
+                continue
             except _RecoverableTransportError as exc:
                 if self._stop.is_set():
                     # ``close`` interrupted the request on purpose. Counting it