feat(client): Agent Skills — a distinguishable outcome for integrity failure (Gap 2 / LA-2)

[XieX]

Gap 2 (finding LA-2): a distinguishable outcome for integrity failure versus absence

This PR adds public API and needs review as such. Three new exported names:
get_skill_result, SkillOutcome, SkillOutcomeReason.

The defect

get_skill returns None for four distinct outcomes — no such skill, the store raised, the
requested version is not the one held, and content that 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. LA-1 (shipped in #51) gave the
operator a structured log record; this gives the application the same distinction.

The information already existed internally. Resolution in skills_core.py distinguished
all four cases — but only as prose in its error string, which get_skill discarded with
.skill.

The API

SkillOutcomeReason = Literal["absent", "integrity_failure", "ok", "store_unavailable", "wrong_version"]

@dataclass(frozen=True)
class SkillOutcome:
    skill: Skill | None
    reason: SkillOutcomeReason
    detail: str | None

async def get_skill_result(key: str, *, version: int | None = None) -> SkillOutcome: ...

Chosen over a typed exception, an opt-in strict mode, and an init-time callback. detail is
the existing human-readable reason string — already safe to surface (skill key and failure
mode only, never content, never a filesystem path), and a test pins that it stays that way.
get_skill_result raises RuntimeError only when no store is configured, with the same
message as get_skill.

get_skill does not change

Its contract — None for every failure, never raises for one — is documented in its
docstring and in the README, and every existing caller treats None as "no skill". A test
drives all four failures through both accessors: the reason is distinguishable and the
collapsed form still collapses.

Not matched on the error string

Resolution grows a typed reason, set explicitly at every construction site. Deriving the
public reason by pattern-matching Resolution.error is exactly the fragility LA-2 is about,
so the mapping is a table a reviewer can read:

resolve_from_store outcome	reason
the store raised (unavailable=True)	store_unavailable
raw is not a dict	absent
verify_raw_skill returned None	integrity_failure
skill.version != wanted_version	wrong_version
success	ok

reason is declared first and without a default, so a contributor adding a sixth
internal outcome has to decide which public token it maps to rather than inheriting one. That
made the two Resolution sites in skills_fs.py explicit as well — both are the
"could not retrieve" path, both already unavailable=True, both now say
reason="store_unavailable".

Resolution.unavailable is unchanged. It stays load-bearing on the prune path — only a
raising 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.

Three things deliberately not built

• No new telemetry or log record. Gap 1's ld.skills.integrity_failure record already
  fired inside verification before resolve_from_store returned. A test asserts one failed
  retrieval still produces exactly one log record and exactly one signal.
• Gap 1's 8-token IntegrityReasonCode is not threaded into SkillOutcome.
  verify_raw_skill returns None and does not surface which token fired; plumbing it up
  would change that function's return type for a detail the operator already gets from the
  log record. The five-token public reason is the actionable surface.
• No batch equivalents. get_skills and all_skills keep omitting failed entries and
  keep logging the run-level WARN count. A get_skills_result / all_skills_result would
  double the accessor surface for a case nobody has asked for — possible follow-up if a
  customer needs per-key reasons from a batch.

Tests

packages/client/tests/test_skills.py, one new class plus three additions to existing
export/immutability tests:

• one case per reason token — reason, whether skill is populated, non-empty detail
• store_unavailable is distinct from absent: a raising store and an empty store
• every non-ok outcome carries a detail (swept, so a fifth failure path with no message is
  caught by a test whose name says what it is about)
• get_skill still returns None, and still never raises, for all four failures
• get_skill_result raises RuntimeError with no store, asserted equal to get_skill's message
• no second integrity record or signal from get_skill_result
• detail never carries the skill content
• SkillOutcome is frozen; SkillOutcomeReason's tokens and the three new exports are pinned

Docs

• packages/client/README.md — get_skill_result in the Agent Skills API table, a
  fail-closed example under the Gap 1 observability material (exit on integrity_failure,
  tolerate absent), the reason table, and an explicit statement that get_skill is
  unchanged and the two accessors differ only in what they report.
• packages/client/agents.md — the five-token vocabulary, the Resolution → reason mapping,
  and the instruction that a sixth internal outcome must choose a public token rather than
  defaulting to absent.

Lockstep with TypeScript

The type name, the accessor name, and the five reason tokens are fixed across both SDKs; a
sibling PR implements the identical shape in typescript/. Tokens are alphabetical,
matching how IntegrityReasonCode was written on both sides in Gap 1.

Placement

Based on split/skills-self-healing (#57), which is based on #54. A new PR at the top of the
stack so it disturbs none of the existing reviews: Gap 2 spans types.py, skills.py,
skills_core.py and __init__.py, which #50–#53 own, and #52/#53/#54 are approved while
#50/#51 have live reviewer conversation.

Checks

uv run pytest (1481 passed, 11 skipped), ruff check, ruff format --check all clean.
uv run mypy . fails repo-wide on a pre-existing duplicate-conftest error before checking
anything; scoped to the files this PR touches, mypy reports only two pre-existing
unused-ignore warnings that are present on the base commit too.

🤖 Generated with Claude Code

---

Note

Overview
Adds get_skill_result and frozen SkillOutcome / SkillOutcomeReason so applications can tell integrity failure apart from a missing skill, store outage, or version mismatch—without changing get_skill (None for all failures, same path, no extra telemetry).

Internal Resolution now carries a required reason mapped explicitly at each resolve_from_store site (including skills_fs store-unavailable paths); get_skill_result exposes skill, reason, and safe-to-log detail.

README and agents.md document fail-closed handling; tests cover all five reasons, immutability, exports, no double integrity signals, and unchanged get_skill behavior.

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