[XieX]

Draft: the cross-SDK spec for this change is still in review, so details may move. The TypeScript port is happening in parallel in js-ai-sdk.

Why

watch_skills(skill_refs(config), root) is a fixed list: the watcher never re-reads the config. When an AI Config moves pdf-extraction from v2 to v3, v2 stays on disk reported error on every run, and v3 is never written. Unpinning a skill is never seen either.

What changed

All new names are exported from launchdarkly_ai_server.experimental.skills only, and none from the package root.

• SkillSource(refs, config_keys=()) (types.py): a frozen dataclass. refs is a zero-argument callable, sync or async, returning SkillReference values and key strings. config_keys is held as a tuple. A non-callable refs, or a bare string / non-sequence / non-string / empty entry in config_keys, raises TypeError.
• config_skills(key, context) (new skills_source.py): evaluates the config with variation(key, context, None) on every read. Only a disabled variation ([]) or a parsed config (its refs) answers. Every other case raises RuntimeError naming the key and the cause, so it never reads as []: no initialized client, variation raising, the default, a non-object, a missing or non-boolean _ldMeta.enabled, a parse failure, or a malformed skills. It never initializes the client. It reports experimental.configSkills when it is called, not on each read. It projects through a new non-reporting _skill_refs.
• write_skills(source, root) reads the source exactly once, after root validation and before the root is pinned. A raise, a timeout, or a malformed answer is a run-level unavailable retrieval: one empty-key error action under on_unavailable="keep", or a RuntimeError naming the skill source under "raise". Nothing is written or pruned. A malformed answer is None, a string ("*" included), bytes, a non-sequence, or any entry that is not a valid SkillReference or a valid key string. timeout=0 never calls refs. An async source still pending at the deadline is cancelled through asyncio.wait_for. A sync source cannot be interrupted, and the docs say so.
• watch_skills(source, root) registers one flag_tracker.add_listener before the initial reconcile, filtered to config_keys by change.key. The listener only sets the wake flag. The source is read on the watcher's worker thread, never on the base SDK's synchronizer thread, which holds the store lock. Store and config triggers share one debounce. With non-empty config_keys, watch_skills raises with both remedies named in these cases: no client, no flag_tracker.add_listener, or is_offline() returning true or raising. The store listener is detached first. close() calls flag_tracker.remove_listener when the tracker has one. A fixed list and "*" never probe or subscribe.
• SkillWatcher.notify() is documented as public: it schedules a reconcile through the same debounce, for a source with config_keys=().
• Docs: the README gets a per-form table of what reaches disk ("*" / fixed list / config_skills), a Following an AI Config's changes section (failed reads, evaluation events, the change-signal requirement, daemon mode, hand-built sources), and new export rows. agents.md gets the module row, the documented form, and a new anti-pattern section.
• The API report and the hand-written usage row (experimental.configSkills) are updated.

Tests

tests/test_skills_source.py (117 tests): construction, every config_skills classification row, and the None default. It also checks that the client is not initialized even with LD_SDK_KEY set, that each read is fresh, and the usage and telemetry rules. write_skills coverage: source/list equivalence, read-once on a raising source, 15 malformed answers in both keep and raise, and None after two writes leaving both on disk. Also an invalid entry failing the read rather than being dropped, timeout=0, and cancellation. Watcher coverage:

• the no-subscribe forms, every probe failure, and notify()
• one read per trigger kind, unrelated keys not waking the watcher, listener-only-signals (read on a different thread), and registration before the initial reconcile
• the mixed-trigger debounce, close detaching
• version bump in both arrival orders (no removed), dropping a reference, and answer → raise → answer

The client is a hand-built fake, not a MagicMock. One test drives the real base SDK: TestDataV2 registered as initializer and synchronizer through ldclient.datasystem.custom(). td.update() bumps v2 → v3, and the test waits on the watcher's reconcile counter. That test passes on the locked ldclient 9.18.3.

make lint format-check typecheck api-report-check passes. uv run pytest packages/client tests passes: 1987 passed, 18 skipped, and 5 repeat runs of the threaded modules were green. One test in the full uv run pytest fails locally: packages/opencode-agents/tests/test_handler.py::TestProcess::test_a_fake_opencode_is_started_spoken_to_and_stopped. It is a path-resolution assertion in a package this PR does not touch.

Where the spec was unclear or wrong

1. Resolved: an absent reference does not hold the prune of a dropped skill. The spec first said a dropped skill stays on disk while another reference in the same answer is absent. It now matches the reconcile in both SDKs: absent is a per-key outcome, and only an unavailable retrieval (a store that raised, no initial data, an exhausted timeout) holds the prune. TestDroppingAReference covers all three cases: a clean drop removes; a drop beside an absent reference still removes and keeps the absent skill's file; a drop beside an unreadable reference does not remove until the store answers.
2. "A mixed burst adds exactly one reconcile." Python's watcher clears the wake flag before the debounce window, so a burst spread across the window gives two reconciles. The existing store-only coalescing test already pins two. The mixed-trigger test asserts the same two reconciles and two reads. This divergence predates this PR.
3. ldclient.datasystem.custom() needs import ldclient.datasystem. A plain import ldclient does not expose the submodule.
4. Usage reporting. The other Agent Skills helpers do not report usage in Python yet, so experimental.configSkills is the only Agent Skills row. write_skills and watch_skills therefore trivially report nothing for a source's reads. _skill_refs is in place for when skill_refs starts reporting.
5. What counts as a malformed answer. The spec says "a sequence holding anything else". I read invalid key strings, invalid SkillReference keys or versions, Skill values, sets and dicts as malformed. An explicit list in the spec would settle it.
6. Error classes, now matching TypeScript. A non-sequence config_keys (a set or generator) and an empty key both raise TypeError, as createSkillSource does. The spec only names the bare-string and non-callable cases, so it should list these two.
7. Base SDK version. The spec's base-SDK checks cite ldclient 9.16.0; the lock pins 9.18.3, where the behavior matches (FlagChange fires on the FDv2-synchronizers thread).

Will conflict with #210, which promotes Agent Skills to the package root. Whichever lands second moves SkillSource / config_skills too.

🤖 Generated with Claude Code

---

Note

Overview
Adds SkillSource and config_skills(key, context) so write_skills / watch_skills can re-resolve which skills an AI Config pins on every reconcile, instead of freezing skill_refs(config) at startup.

write_skills now accepts a SkillSource: it reads refs once per run (sync or async, bounded by timeout), and treats raises or malformed answers as a run-level unavailable retrieval—no writes and no pruning—so a failed evaluation never looks like “revoke everything.”

watch_skills wires the skill store listener together with the base client’s flag_tracker for the source’s config_keys, waking only from the listener thread and re-reading the source on the watcher worker. It raises when config follow-up is impossible (no client, no flag_tracker, offline); fixed lists and "*" behave as before.

Docs and agents.md spell out the "*" vs fixed list vs config_skills disk behavior; exports and API report are updated, with broad tests including a real ldclient TestDataV2 path.

^{Reviewed by Cursor Bugbot for commit 25f9326. Bugbot is set up for automated code reviews on this repo. Configure here.}

[jeffdupont]

Review comment (not an approval). Checked against spec launchdarkly/ai-sdks-monorepo#37 (TESTING.md on xie/watch-skills-follow-config), js-ai-sdk#162, and the skills-to-core PRs (#210 here, js#159, spec #86). CI is green; no review threads yet.

1. absent does not hold the prune: confirmed, and JS does the same. Fix the spec, not the reconcile

• On main, _resolve_requests sets incomplete only for an unavailable reference (skills_fs.py:469-475 on this branch, the same text on main). An absent reference stays requested with an error action, so the reconcile still prunes the dropped key. test_an_absent_reference_does_not_hold_the_prune (tests/test_skills_source.py:1049) pins that.
• js-ai-sdk main has the same rule (packages/client/src/skills-fs.ts:413-415). js#162 tests it the same way. The two SDKs agree with each other.
• Spec #37's "Dropping a reference prunes on the next complete reconcile" bullet says an absent second reference keeps the dropped skill on disk. That contradicts §3.22's per-key bullet ("A withheld skill is protected by its key, not by suppressing the prune … the narrowness is the point"). It also leans on §3.22's ambiguous "a reference that did not resolve".
• Fix: before this leaves draft, get #37 to change "is absent" to "is unavailable (the store raised, no store, uninitialized store, exhausted timeout)". In the same edit, get §3.22's "a reference that did not resolve" reworded to "a reference whose retrieval was unavailable". Keep the reconcile as it is. Changing it would let one not-yet-delivered pin block every other cleanup.

2. The spec's "incomplete" half has no test here (JS has one)

• TestDroppingAReference (tests/test_skills_source.py:1012) covers the clean prune and the absent case. It does not cover the half #37 asks for: a second reference that cannot be resolved holds the prune, and the run after it resolves prunes.
• Fix: add the test js#162 uses (skills-source.test.ts, "prunes it on the next complete reconcile, and not while another reference is unavailable"). Use a store whose get_object raises for c. The source drops b and adds c. Assert b stays on disk with no removed. Then make c readable, fire store.put, and assert b is removed.

3. The mixed-trigger debounce gives 2 reconciles; the spec and JS give 1

• SkillWatcher._run clears _wake before the window (skills_watch.py:137). A burst spread across the window therefore runs a second pass. test_the_debounce_spans_both_trigger_kinds asserts reconciles == 2 (tests/test_skills_source.py:883). Spec #37 says "adds exactly one reconcile and exactly one read". §3.26 already says "exactly one reconcile per debounce window". JS restarts its timer on every notify() (skills-watch.ts notify), so it gives 1.
• Fix (either one): (a) in a separate fix(skills) PR, make _run a trailing debounce: after self._stop.wait(self._debounce), loop while self._wake.is_set() (clear, then wait again) before _reconcile_once(), and change both tests to == 1. Or (b) get #37 to record the Python behavior as an allowed difference. Leaving it as is means a test that contradicts the spec text.

4. An is_offline() that raises skips the remedy message (JS treats it as offline)

• skills_watch.py:365-366 calls is_offline() without a guard. If it raises, the raw exception escapes watch_skills without the "both remedies" text. JS catches it and treats the client as offline (skills-watch.ts:218-223).
• Fix: try: offline = is_offline() is True / except Exception: offline = True, then raise the existing offline RuntimeError. Add one test with a fake whose is_offline raises.

5. Error classes differ from JS for an empty key

• An empty config_keys entry raises ValueError (types.py, SkillSource.__init__). JS raises TypeError for the same input (types.ts createSkillSource), and so does configSkills(''). A.12's table records only the bare-string case.
• Fix: pick one class in both SDKs, or record the split in A.12. Your PR body item 6 already asks this; add it to #37 before merge.

6. Interaction with #210 (Agent Skills to core) and spec #86

#210 deletes experimental/skills.py, its API report and the skills rows in changelog-sections. Spec #86 moves every skills usage ID to client.*. Spec #86 lists seven IDs, and #37 lists eight under experimental.*, so the two spec PRs conflict too. If #210 merges first, this PR needs to:

• export SkillSource / config_skills from launchdarkly_ai_server/__init__.py and drop the experimental/skills.py hunk;
• run make api-report, so the two names land in api-report/launchdarkly_ai_server.txt;
• change report_usage("experimental.configSkills") (skills_source.py, first line of config_skills) and the test_sdk_usage.py row to client.configSkills;
• move the test_skills.py export assertion to the root-surface test;
• retitle away from feat(skills):. After #210 that scope no longer maps to Experimental, so use a core scope such as feat(client):.

Spec #37 has to be rebased over #86 in the same way (usage-ids.txt, §3.27's count, the "§3.19–§3.26a … experimental" marker). Otherwise the synced lists will disagree with this PR.

Parity checked with no gap found

config_skills classification table and None default; no lazy init; usage reported on the call, not on reads; _skill_refs used for the reads; a source read exactly once per run; malformed answers (an invalid entry fails the whole read); timeout=0 never calls refs; async cancel at the deadline; listener registered before the initial reconcile; the listener only signals; no-signal raises naming both remedies, with the store listener detached; close() detaches; "*" and lists never probe; version bump in both orders with no removed; a real-SDK test through TestDataV2.

Not checked: I did not run the suite locally, and I relied on CI and the diff.