feat(client): Agent Skills — the FDv2 delivery transport

[XieX]

Stacked on #66 (split/skills-review-closeout), the tip of the Agent Skills stack. Review that first.

Builds the real delivery transport behind the existing SkillStore interface, on LaunchDarkly's SDK-facing FDv2 channel. The transport itself needed nothing above the interface — the accessors, integrity verification, and write_skills are untouched. Two small additive changes above the line came along with it, both described under Above the interface below.

Why this shape

Skill content arrives over GET /sdk/poll and GET /sdk/stream, authenticated with the environment's server-side SDK key. These are the genuinely SDK-facing endpoints — the same ones the base SDK's FDv2 data source uses, and the channel that payload signing will eventually cover. No private or internal route is involved, and no credential other than the environment's own SDK key ships to a customer host.

What's here

skills_fdv2.py — FDv2SkillStore. Authenticates, streams (default) or polls, carries selector/basis across requests, sends If-None-Match and treats 304 as a first-class current answer, deserialises inline-resource/skill objects, holds them by (key, objectVersion), and serves get_object / all_objects / add_listener / remove_listener. Capped jittered backoff; Retry-After honoured but clamped to max_backoff and rejected when non-finite; bounded consecutive-failure retries, where a committed payload resets the count. Request timeout follows the mode — 10s for a poll, 300s for a stream, since a stream is meant to sit idle. Standard library only, so the content path adds no dependency to a package whose sole runtime dep is opentelemetry-api.

skills_watch.py — watch_skills / SkillWatcher. The change-listener re-reconcile, detaching from the store on close.

Above the interface

Two additive changes, neither of them in the retrieval or reconcile path:

• remove_listener(kind, fn) on the SkillStore interface and on InMemorySkillStore. SkillWatcher.close() needs to detach, and without it a store held every watcher ever created for the rest of its life. Optional in the same way add_listener is: the watcher probes for it and skips detaching when a store does not implement it, so a customer's own store keeps working.
• NO_STORE_MESSAGE now names FDv2SkillStore first and keeps InMemorySkillStore as what it is. It is the first thing a user sees on a missing store, and offering only the development store was wrong once a production transport existed.

Three things worth reviewing closely

1. objectVersion is the skill's version; version is the payload's. On the wire a skill put-object carries both, and confusing them fails silently — the object verifies, the hash matches, and the caller gets content under a version number that means nothing. Flags and segments carry only version and omit both category and objectVersion, which is exactly why the two look interchangeable. The translation happens in exactly one place (_store_object_from_put) and TestVersionTranslation asserts it in both directions, including end-to-end.

2. Changes commit at payload-transferred, not per object. 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, which is also the granularity the re-reconcile wants.

3. A hashless object is held, not dropped. 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. Holding it means verification withholds it with missing_content_hash, which is diagnosable. See the open items below.

Security and safety properties

• Revocation no longer waits for a process restart. A delete-object reaches a live stream in seconds, so watch_skills gets a revoked skill's SKILL.md off disk within a debounce interval instead of at the next restart. on_unavailable="keep" stays the default: an outage must not read as "everything was revoked".
• Server-side only. A mobile key or client-side environment ID raises from the constructor. Payload assignment is shared across credential types, so a client-side credential may well succeed against these endpoints and deliver customer-confidential content to a client-side process. This is the SDK-side half of that boundary; excluding skills at assignment time is the platform-side half and remains an open ask.
• Unknown kinds are ignored, not rejected. An environment's assignment carries its flag payload alongside its agent-skill payload, so flag and segment objects arrive as a matter of course. Erroring on them would turn a normal payload into a permanent reconnect loop — a flag-delivery outage caused by a skills rollout.

Bugs found and fixed while testing

Five, all in the delivery loop, and all sharing one shape: the store stopped delivering while continuing to report itself healthy.

• The consecutive-failure counter never reset in stream mode. _stream_once always ends by raising, so the reset in _run was unreachable and failures grew for the whole process lifetime. Eleven fully successful payload transfers were enough to trip max_consecutive_failures and stop delivery for good — including revocations, which defeats the whole point of streaming. A commit now resets the count. This is the one to read the diff for.
• A non-finite Retry-After killed the delivery thread. float("inf") parses, and Event.wait(inf) raises OverflowError from inside the recoverable-error handler, where the sibling except Exception cannot catch it. The thread died with failed still None. Non-finite values are now rejected and every honoured delay is clamped to max_backoff.
• close() during the initial connect waited out its full join timeout. self._connection was assigned after the connect returned, so a close() in that window found nothing to interrupt. Now re-checked immediately after the assignment.
• close() blocked for the full join timeout on every healthy stream. The delivery thread parks in a socket read no flag can reach, and closing a urllib response from another thread does not unblock CPython's buffered reader. _interrupt_read shuts the socket down underneath it.
• A stream interrupted by our own close was reported as a delivery failure.

Also fixed while cleaning up: the hashless-object ERROR deduped per process rather than per store, so a second store in one process was silently quieter than the first — and that error is the loudest signal of the contentHash gap below. And connect_timeout was accepted and never used, so a poll against a black-holed host hung for 300s rather than 10; it is gone, with the request timeout now chosen by mode.

Tests

141 tests: 133 in test_skills_fdv2.py against _FakeFDv2Endpoint, an in-process ThreadingHTTPServer implementing the wire contract — real sockets, so request construction and header handling are exercised rather than mocked — plus 8 in test_skills_watch.py, which drives the watcher through the interface rather than through a transport. Covers skill put/delete, objectVersion vs version, mixed payloads where flag and segment objects are skipped, unknown kinds ignored, 304, basis round-tripping, reconnect/backoff in both modes, Retry-After including non-finite and oversized values, bounded retries and the reset on commit, prompt shutdown during connect and during a healthy stream, listener detachment, interface parity with InMemorySkillStore, and a missing-contentHash envelope producing withheld skills with the correct reason code rather than a crash.

Full suite 1629 passing, 11 skipped; ruff, ruff format, and mypy clean.

Open items — none of these are in this PR's scope

• 🔴 contentHash is not on the wire yet. The delivered envelope is still {contentType, content, name, description}. Verification withholds any skill without a hash, so against a real environment today every skill resolves to nothing. This PR makes that outcome loud rather than surviving it: an error per (key, version) naming missing_content_hash, one summary per wholly-hashless payload, and a StoreDiagnostics.hashless_objects counter. There is deliberately no fallback that skips verification — a hash the SDK computed from the content it was handed would certify the content against itself.
• 🔴 Server-side inline-resource delivery is not deployed. No account can receive skill objects at all until it ships.
• 🟡 FDv2 is opt-in per account and off by default. A real environment returns 403 today. The store reports that as fatal and explains what to do, but the path is only exercised against the fake endpoint.
• 🟡 mv is a guess. The base SDK's own FDv2 data source doesn't send mv at all, so I could not observe what the server expects. Defaulted to 1 and overridable via constructor. This is the most likely thing to be wrong on first contact with a real server — needs confirmation before Beta.
• 🟡 No payload signing on this channel yet, so Beta is TLS-only; the content hash establishes self-consistency, not origin authenticity.
• 🟡 The connection also carries the environment's flags — no way for a client to request only the skill payload. Skipped cleanly and counted, but the confidentiality and payload-scale concern is a transport property, not fixable here.
• 🟡 ld-relay does not speak the FDv2 endpoints, so relay-only deployments cannot receive skills in Beta.

Nothing here has touched a real LaunchDarkly environment, because it cannot yet. Everything is verified against the fake endpoint.

Known inconsistency, deliberately not fixed here

The modules in this PR say "interface"; skills_core.py below it still says "seam" a dozen times, including a section header and SKILL_OBJECT_KIND's docstring. Converting reviewed code in the lower half of the stack belongs in its own change, so the feature reads both ways until then.

🤖 Generated with Claude Code

[XieX]

This has been split into smaller PRs, so I'll close it out in favor of them,

#81
#82
#83