feat(client): Agent Skills store seam, verification, and accessors

[XieX]

Stacked PR 3 of 7 — merge bottom-up into xie/agent-skills-feature-ac9ac7.

1. feat(client): Agent Skills value types #29 — value types
2. feat(client): symlink-refusing filesystem primitives #30 — symlink-refusing filesystem primitives
3. feat(client): Agent Skills store seam, verification, and accessors #31 — store seam, verification, accessors ← you are here
4. test(client): integrity verification and the accessor telemetry sweep #32 — integrity + telemetry tests
5. feat(client): writeSkills materialization and the abuse matrix #33 — writeSkills + abuse matrix
6. test(client): the rest of the writeSkills suite #34 — remaining writeSkills tests
7. docs(client): document Agent Skills #35 — docs

Each PR targets the one below it, so GitHub already shows only this PR's own diff.

---

Third of seven. The whole non-filesystem runtime, and the first PR in the stack where the feature does something end to end.

Layering

skills-core.ts holds what the two layers above it share: the SkillStore and telemetry seams, the module state behind them, integrity verification, and store resolution. It imports neither of those layers.

Keeping the store and the emitter here is what makes it impossible for the accessor layer and the filesystem layer to disagree about whether one is configured. The dependency edge that would close that cycle must not be added.

skills.ts is the public half: skillRefs, getSkill/getSkills/allSkills, InMemorySkillStore, and the documented injection points.

lifecycle.ts accepts skillStore on both initClient overloads. The BYOC overload gains an options argument, which is how an edge-runtime caller configures one. It's applied before the idempotency check and on every call, so a client that initialized lazily — or without a store — can be given one afterwards. A nullish value never clears a configured store. shutdown() clears the skills state unconditionally and ahead of its own early return, because that state can exist without a client.

The wire string is encoded exactly once, here

The wire object delivers content as a JSON string. verifiedBytes encodes it to UTF-8 bytes exactly once (TextEncoder), enforces the size cap on the encoded bytes, hashes the bytes (sha256, lowercase hex — byte-identical with the Python SDK, never a string API), and the verified bytes are what the Skill carries. "Skills are opaque byte buffers" is true by construction from here on: nothing downstream re-encodes, decodes, or interprets them. The same verifiedBytes accepts a Uint8Array for the pre-write pass in #33, where a Skill's bytes are hashed as-is.

Content delivery is not wired up

Everything runs against the SkillStore seam and the shipped default is absent, so the accessors throw an actionable error until one is configured. InMemorySkillStore covers local development, tests, and bring-your-own-content. The real transport drops in behind the same interface without touching the public API.

Trust boundary

Nothing a store serves is trusted. The transport is not part of the trust boundary, so key, version, size, and content hash are revalidated at the accessor boundary on every pass. A Skill only exists once verification has passed — and writeSkills will re-verify anyway, because a Skill can also be built by a caller.

Telemetry

Signals go through a private no-op emitter and never client.track(). These are LaunchDarkly product-analytics signals, not customer analytics: track() would require an LD context, spend the customer's event volume, and land in their data export.

Exactly three signal names exist and they are an allowlist, not a floor. No skill content and no filesystem paths reach a signal — hashes and byte counts only.

One defense worth reviewing closely

The UTF-8 round-trip guard in verifiedBytes is invisible from the outside and could be deleted with every other test still green. TextEncoder silently substitutes U+FFFD for an unpaired surrogate where Python raises — so without the guard, a store supplying the sha256 of the substituted bytes has fabricated content pass verification. The guard runs only on the wire-string path; bytes handed in directly are already just bytes.

Its test (in the next PR) pins contentHash to exactly that hash, so the hash comparison is provably not what rejects it.

getSkillResult — a distinguishable outcome for tampering (security review LA-2)

getSkill resolves to null for four distinct outcomes: no such skill, the store threw, the requested version is not the one held, and content failed hash verification. A customer therefore cannot fail closed on suspected tampering while tolerating a merely-absent skill, so no automated customer-side response is possible. The information already existed internally — Resolution distinguished all four — but only as prose in its error string, which getSkill discarded.

getSkillResult(key, { version? }) resolves to { skill, reason, detail }. skill is non-null exactly when reason is 'ok'; detail is the existing human-readable reason string, which carries no skill content and no filesystem path and must stay that way.

getSkill is unchanged. Its documented contract — "resolves to null, never rejects; throws only when no store is configured" — is frozen, because every existing caller treats that null as "no skill". getSkillResult shares the identical lookup, the identical verification, and the identical single throw; the two differ only in what they report. #32 asserts getSkill still resolves to null for all four failures.

Resolution gains a typed reason field, set explicitly at every construction site rather than derived from error — pattern-matching a prose string to decide what a customer's fail-closed branch sees is precisely the fragility this finding is about. The field is required, so a sixth internal outcome has to choose a public token rather than inherit 'absent' by omission. The five sites map 1:1:

resolveFromStore outcome	reason
the store threw (also sets unavailable)	store_unavailable
raw is not an object	absent
verifyRawSkill returned null	integrity_failure
skill.version !== wantedVersion	wrong_version
success	ok

unavailable stays as it is. It is load-bearing on the prune path — only a throwing store suppresses pruning, because deleting managed files after a failed lookup would turn an outage into data loss — and store_unavailable stays distinct from absent for the same reason.

The store seam now carries the version

resolveFromStore previously accepted a wantedVersion and then called store.getObject(kind, key) without it, relying on a post-hoc equality check. That worked while the reason was invisible; publishing a typed wrong_version on top of it would report the wrong reason for a customer store that holds several versions of a key and would have answered the pin correctly if asked. The version is now threaded through, and the equality check is kept as a defense rather than as the selection mechanism, because the store is untrusted.

InMemorySkillStore accepts the parameter and holds one object per key, so it answers with what it has and lets the accessor refuse a mismatch — deliberately not a filter, since returning null for an unsatisfiable pin would report a version mismatch as an absence. Giving it real multi-version semantics is a separate change that pulls in allObjects and allSkills.

Deliberately not built

• No new telemetry and no new log record. The ld.skills.integrity_failure record already fired inside verification before the resolution returned; recording anything here would double-log one failure. test(client): integrity verification and the accessor telemetry sweep #32 asserts exactly one record per failed retrieval.
• No IntegrityReasonCode in SkillOutcome. verifyRawSkill returns null and does not surface which of the eight tokens fired. Plumbing it up would change that function's return type for a caller-facing detail the operator already gets from the log record.
• No batch equivalents. getSkills and allSkills keep omitting entries they could not return. Possible follow-up: a reporting form for the batch accessors, if a caller turns out to need per-entry reasons across a whole reference set rather than per key.

Verification

Client package 355 → 395 tests. typecheck and biome clean.

🤖 Generated with Claude Code

---

Note

Overview
Adds the Agent Skills runtime for retrieving verified skill content through an injectable SkillStore, without wiring LaunchDarkly delivery yet.

New modules: skills-core.ts centralizes global store/telemetry state (on globalThis), SHA-256 integrity verification with ld.skills.integrity_failure logging, and shared resolveFromStore. skills.ts exposes skillRefs, getSkill / getSkills / allSkills, getSkillResult (typed reason for tampering vs absent vs store outage), and InMemorySkillStore for local/BYOC use.

Lifecycle: initClient accepts skillStore on both overloads (applied before client idempotency; omitting it does not clear an existing store). shutdown() always clears skills state.

Trust model: Wire content is UTF-8–encoded once; failed verification is omitted from batch results and emits integrity telemetry (no skill bodies in signals). Store lookups pass version into getObject, with a post-check defense for untrusted stores.

Tests expand skills.test.ts with OTel mocks, store lifecycle, accessors, and integrity signal coverage. Public exports added in index.ts.

^{Reviewed by Cursor Bugbot for commit 8d4bad2. 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 1 potential issue.

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

[andrewklatzke]

Would be great to add a test regarding this:

The UTF-8 round-trip guard in verifiedBytes is invisible from the outside and could be deleted with every other test still green. TextEncoder silently substitutes U+FFFD for an unpaired surrogate where Python raises — so without the guard, a store supplying the sha256 of the substituted bytes has fabricated content pass verification. The guard runs only on the wire-string path; bytes handed in directly are already just bytes.