Skip to content

fix(client): a broken store answer is not an absent skill - #57

Merged
XieX merged 2 commits into
xie/skills-08-review-closeoutfrom
xie/skills-store-answer-hardening
Sep 15, 2026
Merged

XieX merged 2 commits into
xie/skills-08-review-closeoutfrom
xie/skills-store-answer-hardening

Conversation

@XieX

@XieX XieX commented Sep 11, 2026

Copy link
Copy Markdown

Ports two Agent Skills store-answer fixes from launchdarkly/python-ai-sdk (split/skills-materialization, launchdarkly/python-ai-sdk#53) into the Node client.

What changed

A non-object listing is a broken store, not an empty one (packages/client/src/skills-core.ts). allRawObjects used to collapse anything store.allObjects() returned that was not an object down to {} with no error. A store that served nothing usable was therefore indistinguishable from a store holding no skills, which the '*' reconcile read as "every skill was revoked" and pruned accordingly. It now returns { objects, error }, logs an error naming the returned type (the skill store listed skills as null rather than an object), and puts that answer on the same footing as a store that threw. A throwing store is caught in the same place with the same wording (storeThrew), so allSkills and the reconcile's resolveAll no longer each re-derive the log line and the message. allSkills still returns [] on either failure; writeSkills('*') reports an incomplete run and prunes nothing. Mirrors Python's list_raw_objects returning (objects, error).

An answer under a different key is withheld (resolveFromStore). Identity is read off the object itself, so a store answering under a different key used to hand the caller a Skill carrying someone else's key. The reconcile then wrote that other key's path and, since prune keys off the request, deleted it in the same pass while reporting ok. After verification, skill.key !== key is now withheld with skill '<key>' is not available: the store answered under key '<skill.key>', using the same shape as the existing version-mismatch branch.

Tests. Accessor side from Python 5817d89 (aliased key withheld by getSkill / getSkillResult; non-object listing reported via allRawObjects with the type named) and reconcile side from 4c6d965 (a non-object listing leaves every managed file and manifest entry alone; an aliased answer writes nothing, is reported against the requested key, and never reaches the other key's file). All six fail against the unpatched source.

Commits mirror Python 5817d89 and 4c6d965.

Notes for review

  • Reason token for the key mismatch. Python's Resolution has no typed outcome, so there is nothing to port there. Here it reuses wrong_version, per the existing branch's shape: the store held an answer, but not the one that was asked for. If a dedicated token is wanted it is a cross-language vocabulary change and belongs in its own PR.
  • e2b54fd (an unheld version pin is a miss, not an integrity failure) is deliberately not ported. Python's InMemorySkillStore files several versions per key plus a version-less slot, and that commit changes which one a pinned miss falls back to. Node's InMemorySkillStore holds exactly one object per key and ignores the version argument, letting resolveFromStore refuse a wrong-version answer afterwards. There is no fallback slot to fix, so the change has no direct equivalent; whether Node should grow multi-version semantics is a separate decision.
  • feat(client): Agent Skills — the FDv2 delivery transport #50 (xie/skills-fdv2-transport) sits above this branch and will need a rebase once this lands, since allRawObjects changed its return type and resolveAll / allSkills changed accordingly. Not rebased here.

Verification

  • vitest run in packages/client: 652 passed, 2 skipped
  • tsc --noEmit: clean
  • biome check: clean

🤖 Generated with Claude Code


Note

Overview
Hardens the Node client against untrusted skill store behavior (ported from the Python SDK): broken listings and key aliasing no longer look like “no skills” or succeed while corrupting disk state.

Non-object listingsallRawObjects now returns { objects, error } instead of silently {}. Throws and invalid list shapes (null, array, etc.) are logged with consistent wording via storeThrew, and writeSkills('*') treats a listing error as an incomplete run so prune does not delete managed files. allSkills still returns [] on failure but the error is available on the listing path.

Key aliasing — After verification, resolveFromStore withholds answers where skill.key !== the requested key, with the same outcome shape as version mismatch (wrong_version). That blocks getSkill from returning the wrong identity and stops writeSkills from writing under one key and pruning under another in the same reconcile.

Tests cover accessor and filesystem reconcile paths for both fixes.

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

XieX and others added 2 commits September 14, 2026 16:09
Two untrusted-store answers were read as data rather than as failures.

`allRawObjects` collapsed a non-object listing to `{}` with no error, so
a store that served nothing usable was indistinguishable from one holding
no skills. `resolveFromStore` read identity off the object without
checking it against the key that was asked for, so an answer served under
a different key came back under the caller's key while carrying its own.

Both are now withheld and reported, alongside the version check that
already guarded the same way. `allRawObjects` now returns the listing
together with the reason it could not answer, with a throwing store
caught and worded in the same place, so `allSkills` and the `'*'`
reconcile path no longer each re-derive the log line and the message.

Ports launchdarkly/python-ai-sdk 5817d89.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Covers the reconcile side of the withheld-answer fix: a listing that is
not an object leaves every managed file alone rather than reading as a
full revocation, and an answer served under a different key writes
nothing, is reported against the key that was asked for, and does not
reach that other key's file.

Each one previously deleted a file and reported a clean run.

Ports launchdarkly/python-ai-sdk 4c6d965.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@XieX
XieX force-pushed the xie/skills-08-review-closeout branch from 90b45a5 to 888a616 Compare September 14, 2026 20:13
@XieX
XieX force-pushed the xie/skills-store-answer-hardening branch from 5657291 to ee5d469 Compare September 14, 2026 20:13
@XieX
XieX merged commit 843f522 into xie/skills-08-review-closeout Sep 15, 2026
8 checks passed
@XieX
XieX deleted the xie/skills-store-answer-hardening branch September 15, 2026 20:28
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.

1 participant