Skip to content

feat(skills)!: promote Agent Skills to core - #210

Open
jeffdupont wants to merge 7 commits into
mainfrom
jdupont/aic-3655-skills-core
Open

jeffdupont wants to merge 7 commits into
mainfrom
jdupont/aic-3655-skills-core

Conversation

@jeffdupont

@jeffdupont jeffdupont commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Promotes Agent Skills from launchdarkly_ai_server.experimental.skills to the package root. Jira: AIC-3655.

Do not merge yet. This PR must not merge until the blockers linked on AIC-3655 are fixed. It releases together with the JS and Go promotion PRs (links to be added).

Migration

Import Agent Skills names from launchdarkly_ai_server instead of launchdarkly_ai_server.experimental.skills. Nothing else changes: the names, signatures and behaviour are the same.

# before
from launchdarkly_ai_server.experimental.skills import get_skill, write_skills
# after
from launchdarkly_ai_server import get_skill, write_skills

This is a breaking change (!), which bumps the minor version while the package is 0.x.

What changed

  • Root exports. All 24 names that experimental/skills.py exported are now in launchdarkly_ai_server.__all__, same spelling. launchdarkly-ai-python re-exports root __all__ with import *, so it gets them with no code change; its existing re-export tests cover it.
  • Experimental module removed. experimental/skills.py is deleted. experimental/__init__.py is only a docstring, so nothing else there referenced it. Evaluations from Code stays in experimental.evaluations, untouched.
  • API reports. Regenerated with make api-report: the skills names appear in launchdarkly_ai_server.txt, and launchdarkly_ai_server.experimental.skills.txt is deleted.
  • Changelog. The three skills → Experimental rows are removed from changelog-sections in release-please-config.json, so feat(skills): now lands under Features.
  • Docs. The client README, the launchdarkly-ai-python README, packages/client/agents.md and AGENTS.md no longer call Agent Skills experimental. Code samples import from the root.
  • Core strings. Docstrings and error messages in skills_core.py, skills_watch.py, types.py, types_validation.py, shutdown_hooks.py and lifecycle.py no longer name the experimental module. For example, the "no skill store" error now says set_skill_store(store) from launchdarkly_ai_server.
  • Usage IDs. None. main reports no skills usage IDs today, so this PR adds none. When skills usage reporting is added, it must use client.* IDs, not experimental.*.
  • Unchanged behaviour. parse_ai_config still passes skills through unvalidated, and skill_refs still validates it. shutdown() still clears the skill store through the internal shutdown hook. The lifecycle module still imports no skills module.

Tests

  • test_skills.py: every skills name is in root __all__ and on the root module. launchdarkly_ai_server.experimental.skills raises ImportError, and launchdarkly_ai_server.experimental has none of the names.
  • test_public_surface.py: the skills names are added to the ROOT_SURFACE snapshot.
  • test_lifecycle.py: the "core loads no experimental module" check now checks evaluations and experimental only, since the root now imports the skills modules.
  • The skills test suites and conftest.py import from the root.

Test plan

Run in the PR worktree after uv sync (exit 0):

  • make lint — exit 0
  • make format-check — exit 0
  • make typecheck — exit 0 (Success: no issues found in 101 source files)
  • make api-report-check — exit 0 (14 API reports are current)
  • uv run pytest -q packages/client packages/ai — 1709 passed, 1 skipped
  • make test — exit 2: 3282 passed, 18 skipped, 3 failed. The same 3 tests fail on unmodified origin/main (e3afb3f) in the same checkout, and CI on main is green. None of them involves skills:
    • test_lifecycle.py::TestInitClientSDKKeyPath::test_uses_options_sdk_key_over_env_var and ::test_retries_after_a_failed_init_instead_of_replaying_it: they get a staging stream_uri. My unverified guess is a local .env above the checkout leaking into the full run. Both pass when run alone.
    • opencode-agents/tests/test_handler.py::TestProcess::test_a_fake_opencode_is_started_spoken_to_and_stopped: on macOS the temp path comes back as /var/... in one place and /private/var/... in another.

🤖 Generated with Claude Code

Related PRs

Releases together with:


Note

Overview
Promotes Agent Skills from experimental to the stable package root, so customers import get_skill, write_skills, FDv2SkillStore, and the rest from launchdarkly_ai_server instead of launchdarkly_ai_server.experimental.skills. The experimental re-export module and its API report are removed; the same symbols are added to root __all__ and the generated launchdarkly_ai_server API report.

Release and docs treat skills as core: release-please no longer routes feat(skills): to Experimental (only evaluations stays experimental), and README/agents guidance plus launchdarkly-ai-python docs describe root imports and the launchdarkly-ai-server>=1.0.0 pairing.

Implementation follow-ups include client.* usage reporting on the public skills entry points, an internal _write_skills so watch_skills reconciles without double-counting writeSkills, and updated lifecycle/public-surface tests (including that experimental.skills is gone while evaluations remains experimental).

Reviewed by Cursor Bugbot for commit 75dc0f4. Bugbot is set up for automated code reviews on this repo. Configure here.

jeffdupont and others added 2 commits October 23, 2026 10:25
Every Agent Skills name is now exported from the launchdarkly_ai_server
package root (and so from launchdarkly-ai-python), with the same spelling.
launchdarkly_ai_server.experimental.skills is removed; import from
launchdarkly_ai_server instead. Nothing else changes.

- Root __init__ imports and __all__ gain the 24 skills names.
- API reports regenerated; the experimental.skills report is gone.
- release-please changelog-sections drop the skills Experimental rows.
- READMEs, agents.md, AGENTS.md, core docstrings and error strings no
  longer point at the experimental module.
- Tests import skills names from the root; new tests check the root
  exports them and the experimental module no longer imports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A put or delete with no usable key stays warned and ignored, and delivery
keeps going. Drop the "experimental stage" and "revisit before 1.0"
wording from agents.md and the pinning test's docstring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeffdupont

Copy link
Copy Markdown
Contributor Author

Pushed 4825587, which merges origin/main into this branch with no conflicts. make api-report showed no drift, and make lint-fix and make typecheck are clean. No other changes.

This needs a human reviewer.

@XieX XieX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the promotion, and the code move itself looks clean: no references to experimental.skills remain, and the root __all__ matches the old module's 24 names. My inline comments are mostly about the docs and packaging around it.

One thing outside the diff: the title's scope is AIC-3655. release-please shows the scope in bold in the changelog entry, so the Jira key would appear in the customer-facing changelog. feat(skills)!: promote Agent Skills to core would land under Features in the same way.

Comment thread packages/client/agents.md
Comment thread packages/client/agents.md Outdated
Comment thread packages/ai/README.md
Comment thread packages/client/tests/test_lifecycle.py
Comment thread packages/client/src/launchdarkly_ai_server/experimental/__init__.py
Comment thread packages/client/src/launchdarkly_ai_server/__init__.py
Comment thread packages/client/tests/test_skills.py Outdated
Comment thread packages/client/tests/test_skills_fdv2.py Outdated
jeffdupont and others added 2 commits October 23, 2026 14:11
all_skills, get_skill, get_skill_result, get_skills, skill_refs,
watch_skills and write_skills each report their client.* usage ID as
their first statement (TESTING.md §3.27). The watcher's initial and
follow-up reconciles call the non-reporting _write_skills.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f5ca75f. Configure here.

Comment thread packages/ai/README.md
jeffdupont and others added 2 commits October 23, 2026 14:24
…server 1.0.0

launchdarkly-ai-python doesn't pin launchdarkly-ai-server, so upgrading the barrel alone can leave
an older server installed, and importing a skills name then raises ImportError. The promotion
removed the README warning along with the experimental bullet. This restores it for the root
imports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- agents.md: a rename of ld.skills.integrity_failure needs a ! commit, which is a minor bump on 0.x
  and a major after 1.0, not always a major.
- Test that lifecycle.py imports no skills module, so the claim in agents.md is checked now that
  test_lifecycle no longer asserts it through launchdarkly_ai_server.skills.
- test_skills' SKILLS_SURFACE docstring points at the root snapshot test that actually catches
  an addition.
- test_skills_fdv2 imports the internal skills_core and skills_fdv2 apart from the public names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeffdupont jeffdupont changed the title feat(AIC-3655)!: promote Agent Skills to core feat(skills)!: promote Agent Skills to core Oct 23, 2026

@XieX XieX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. All eight comments from the first round are settled: six are fixed, and the shim and lazy-import decisions are backed by §0.5 and AIC-3674. Thanks for the spec links on the keyless-object rule; ai-sdks-monorepo#86 §3.25 reads the way you described.

Before merging, could you also deal with the follow-ups below?

Docs vs. the new usage reporting (4d6b3cb). The code is right per §3.27, but three places still say skills never calls client.track() or reports telemetry. Details are in the inline comments.

PR description. It still says "Usage IDs. None… this PR adds none." It should now list the seven client.* IDs: skillRefs, getSkill, getSkillResult, getSkills, allSkills, writeSkills and watchSkills.

Changelog. The squash commit only carries the promotion title. Since skills calls now send a usage event, a BEGIN_COMMIT_OVERRIDE with a second line such as feat(skills): report helper usage for the Agent Skills functions would give that its own changelog entry.

Merge timing. This approval doesn't change the "Do not merge yet" note. Please hold until the AIC-3655 blockers are cleared, and make sure this ships as the 1.0 release (AIC-3672). The barrel README now says skills need launchdarkly-ai-server 1.0.0, but the open release PR (#206) would cut 0.5.0. If this lands in a 0.5.0 release, that README note and its pip install "launchdarkly-ai-server>=1.0.0" command would point at a version that doesn't exist.

objects with a valid key and an integer version >= 1. This
includes ``skills: null``.
"""
report_usage("client.skillRefs")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring just above still says "Pure: no network, store, or telemetry", but this call now reports client.skillRefs, and once a client exists that goes out through client.track(). Maybe: "Pure apart from the one-time helper usage event ($ld:ai:sdk:usage): no network, store, or skills telemetry." agents.md line 210 says the same thing ("no client, no telemetry") and needs the same change.

Comment thread packages/client/agents.md
@@ -984,6 +984,6 @@ a conversation is out of reach at this layer either way.
- `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.
- Do not route skills telemetry through `client.track()`, and do not introduce an LD context anywhere in the skills path. Signals go through the `skills_core.py` emitter seam, whose default is a no-op, and only via its `record_*` functions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule, and the Telemetry seam section at lines 586–590 ("Do not use client.track()… No LD context is involved anywhere in this feature"), now contradict the report_usage calls added in 4d6b3cb, which send $ld:ai:sdk:usage through client.track() with SDK_INFO_CONTEXT. The code follows §3.27, so I think the docs should exempt it explicitly. For example: the rule covers the three skills signals; the SDK-wide helper usage event (§3.27, once per helper per client) is the one client.track() path, and it carries no skill data. Otherwise a future contributor following this line could remove the usage reporting as a violation.

@@ -363,7 +366,7 @@ def test_requires_no_client_or_store(self, mock_ld_client: Any) -> None:
mock_ld_client.track.assert_not_called()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the docstring still calls this a "pure projection". track.assert_not_called() passes only because no client is initialized, since skill_refs now holds a usage event for the next init. Could we reword it so it doesn't read as a no-telemetry check? For example: "No store configured, no client initialized: skill_refs still projects; the usage event is held until init (see test_sdk_usage.py)."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants