feat(client): Agent Skills — materialize onto disk under a manifest (4/5)

[XieX]

PR 4 of 5 splitting draft #45 for review. Stacked on #52.

main ← #50 ← #51 ← #52 ← split/skills-materialization ← split/skills-fs-hardening

Important

Aug 28 repivot: skills are now opaque byte buffers by construction. Skill.content is bytes (the verified verbatim bytes, exactly what was hashed), the frontmatter() convenience accessor and frontmatter.py are deleted, and the SDK no longer parses or interprets skill content anywhere. Consumers who want frontmatter parse it themselves. The stack was rebased in place to make each change in the slice that introduced the code; the TypeScript SDK is getting the mirror change (content: Uint8Array) separately.

What's here

write_skills writes <root>/<key>/SKILL.md and records what it owns in a manifest at <root>/.launchdarkly-skills.json, so it overwrites or removes only paths that manifest records. A file you placed yourself is reported and left untouched.

report = await write_skills(refs, ".claude/skills")
for action in report.errors:
    print(f"skill {action.key or '<run>'}: {action.error}")

skills accepts Skill values, references, bare keys, or the literal "*" for everything the store holds. Every outcome is visible in the returned ReconcileReport: one ReconcileAction per skill carrying written, updated, skipped_current, removed or error, plus .ok and .errors. A failure belonging to the run rather than to one skill — an unreadable manifest, a retrieval that failed before any key was known — carries the empty string as its key; callers grouping a report by key need to expect that sentinel.

New exports: write_skills, ReconcileAction, ReconcileReport, ReconcileActionKind, OnUnavailable, SKILL_FILENAME, MANIFEST_FILENAME, MANIFEST_VERSION.

The defenses

Writes are atomic, at mode 0644, and every destructive step runs against a descriptor pinned to a directory that was already checked, so a path swapped after the check cannot redirect a write or an unlink out of the managed root. Where the platform has no *at() family the identical sequence runs against full paths.

• The key is re-validated here regardless of upstream validation, before any filesystem call, because a key becomes a directory name. The data model allows 256 characters and NAME_MAX is 255 bytes, so an over-long key is refused too.
• Never write or unlink through a symlink, on the write path or the prune path.
• Destruction only on manifest-listed paths whose key matches.
• A corrupt manifest fails closed: no overwrites and no prunes, brand-new paths may still be written, an error action names the manifest, and the manifest is not rewritten.
• An incomplete retrieval suppresses pruning, so a transport outage cannot read as "everything was revoked".
• Content is re-verified immediately before the write, because a Skill can also be constructed directly by a caller. With the bytes pivot, this pass hands Skill.content: bytes to the same verified_bytes the accessors use, which hashes the bytes directly — same two-pass design, identical telemetry property keys.

Pruning removes formerly-managed skills that are no longer referenced — that is how revocation takes effect. timeout bounds retrieval, the writes and the pruning; only the final manifest rewrite runs past it, so files already written are never orphaned.

The "*" form collapses to one object per key at its newest version, since <root>/<key>/SKILL.md is a single path and writing it twice in one run is a bug rather than a policy. It reports a withholding count at WARN for the same reason the accessors do (finding 2).

Notes for reviewers

• write_skills blocks. It is async for parity with the other accessors and with the TypeScript SDK, but it awaits nothing: every read, write, fsync and rename runs inline. Wrap it in asyncio.to_thread if that matters on your loop. For the same reason timeout is checked between steps rather than interrupting one already in progress. Reconcile one root at a time — because nothing yields today a run is atomic against the rest of your loop, and wrapping it to run concurrently makes two runs against one root race on the manifest. (Review findings 4 and 6 on feat(client): Agent Skills — umbrella for the #50–#54 split #45 both live here and are tracked there, not resolved in this PR.)
• The root's parent must exist. write_skills creates the root itself but never its ancestors, so a typo cannot scatter a directory tree across a project. An absent parent, a root that is an existing file, and a root that is a symlink each raise ValueError — caller errors, distinct from the per-skill error actions in the report.
• The security abuse matrix is PR 5. Every guard listed above is in this diff; what lands next is the adversary that proves each one fails without it — path traversal, symlink attacks, the directory swap fired at the instant of the rename, clobber protection, corrupt manifests, atomicity under an injected crash, and the telemetry allowlist. This PR's own tests cover the happy path, reconcile semantics, manifest round-tripping, resilience, and verify-before-write. Splitting skills_fs.py itself along that line would have meant shipping a deliberately weakened _write_one/_prune here and hardening it there, which agents.md marks non-relaxable.

Testing

uv run pytest → 1216 passed. ruff check, ruff format --check, and mypy packages/*/src all clean.

🤖 Generated with Claude Code

---

Note

Overview
Adds the materialization layer for Agent Skills: write_skills writes verified content to <root>/<key>/SKILL.md and reconciles against .launchdarkly-skills.json, so overwrites and deletes only touch paths the manifest records under a matching key.

Callers get a ReconcileReport with per-skill ReconcileAction outcomes (written, updated, skipped_current, removed, error), plus report.ok / report.errors. Inputs include Skill values, references, bare keys, or "*" (newest version per key from the store). prune removes formerly managed skills no longer requested (revocation path); pruning is skipped when the manifest is corrupt or retrieval was incomplete, so outages cannot look like mass revocation.

Implementation lives in new skills_fs.py, wired to safe_fs descriptor-pinned atomic writes and skills_core verification/telemetry (record_materialized, record_revoked). Root handling is strict: create only the leaf root, reject symlink/file roots, re-validate keys before any FS call, and re-verify bytes immediately before write.

Public exports expand with write_skills, ReconcileAction / ReconcileReport / ReconcileActionKind, OnUnavailable, and SKILL_FILENAME / MANIFEST_FILENAME / MANIFEST_VERSION. README and agents.md document behavior; test_skills_fs.py covers reconcile semantics, manifest round-trip, resilience, and verify-before-write (adversarial FS matrix noted for a follow-up PR).

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

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{❌ 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 568729d. Configure here.}

[cursor]

Bad listing looks like empty store

High Severity

_resolve_all treats a non-dict all_objects result as a successful empty listing. "*" then looks like every skill was revoked and prune deletes all managed files. That is the same data-loss path the raising-store guard was written to prevent.

 

^{Reviewed by Cursor Bugbot for commit 568729d. Configure here.}

[cursor]

Lookup key and skill key diverge

Medium Severity

_resolve_requests records a reference under the looked-up key, but _write_one writes under skill.key. A store that answers with a different object key writes one path and then prune, which keys off the request, can immediately delete that file.

Additional Locations (1)

• packages/client/src/launchdarkly_ai_server/skills_fs.py#L649-L725

 

^{Reviewed by Cursor Bugbot for commit 568729d. Configure here.}