Skip to content

feat(client): declare ?kinds=agent-skill, and make a 422 a fatal transport failure - #87

Merged
XieX merged 13 commits into
xie/skills-14d-robustnessfrom
xie/skills-kinds-param
Oct 1, 2026
Merged

XieX merged 13 commits into
xie/skills-14d-robustnessfrom
xie/skills-kinds-param

Conversation

@XieX

@XieX XieX commented Sep 24, 2026 •

Copy link
Copy Markdown

Stacks on #67. Spec: ai-sdks-monorepo#23. Python counterpart: python-ai-sdk#109. Server side: streamer#4730.

This is the SDK half of the release blocker: other SDKs retry forever when they see a skill payload. Flag Delivery's answer is ?kinds=, which narrows a connection to the payload kinds it declares, defaulting to {flagging}.

The declaration

Every request now carries kinds=agent-skill — both endpoints, and on the first request as well as the ones after it, since it selects what the connection is served rather than describing what the store already holds. Without it the store receives the environment's flag payload and no skills at all, so this is a functional requirement rather than a courtesy.

It also fixes something that was already wrong. A skill-enabled environment assigns two payloads, so ProtocolReader has been hitting warnMultiplePayloads on every connection, reading only the first intent, and never adopting a basis for the flag payload — re-downloading and discarding it on every reconnect. Declaring one kind makes the connection single-payload, which is the shape the reader is built for. (That is also why declaring flagging,agent-skill is not the safe-looking option it appears to be.)

No mv, still — but for a corrected reason. It selects the flag data model, and objectQueryForCommand overrides whatever a request asks for with the payload's own default for any non-flagging payload, so sending it would state a preference that is ignored. The old comment said the connection would be refused over it.

🤖 Generated with Claude Code, updated by @XieX


Note

Overview
FDv2 skill delivery now sends kinds=agent-skill on every poll and stream request so the connection receives the agent-skill payload instead of default flag data (and avoids multi-payload reader behavior). HTTP 422 is classified as a fatal transport error: delivery stops without retrying, failed/lastError are set, connectionFailures stays at zero, waitForSkills resolves false immediately, and an uninitialized store still blocks wildcard reconcile from pruning disk.

Observability: pinned-version mismatches emit a new SIEM record with reason_code: version_mismatch and served_version (log only, no product signal), parallel to key_mismatch; docs and tests expand the integrity vocabulary from nine to ten tokens.

writeSkills rejects non-finite timeout values (NaN, Infinity) so the reconcile deadline cannot silently disappear.

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

XieX added a commit that referenced this pull request Sep 29, 2026
Stacked on #87 (`xie/skills-kinds-param`), which is where the
`?kinds=agent-skill` work and the current 422 handling live. Review that
first, or read this diff alone — it touches only the 422 classification,
the error class it used, and the diagnostics field.

Conforms this SDK to **ai-sdks-monorepo #29** (`TESTING.md` §3.25 and
A.12), which deliberately leaves both SDKs non-conforming until this
lands. ai-sdks-monorepo #23 specifies the opposite — 422 as a third
class — and is expected to be closed as superseded; this does not
implement it. A parallel task covers the Python side, and the two
messages are kept aligned.

## Why 422 is fatal

LaunchDarkly picked the status to be terminal rather than us inferring
it, and the `streamer` repo says so in both places that emit it — at the
narrow-assignment check
(`internal/fdcore/narrowassign/narrowassign.go:24`) and at the status
constant itself (`internal/fdcore/adapters/httpsrv/status/status.go`) —
each noting the code exists so a misconfigured SDK stops instead of
hammering the fleet. Retrying it forever is the precise behavior the
status was chosen to prevent, so "recoverable" is not a cautious reading
of an ambiguous status; it is the one reading the platform ruled out.

## The stated cause was also wrong

The gate is `payloadvers.SkillDeliveryAllowed` in gonfalon: skill
delivery enabled for the account **and** a credential that is not
view-scoped. Each way of failing it is permanent from the store's side —
the account gate is off (`enable-aic-agent-skills`, default false), the
SDK key is view-scoped, or the declared kind is a typo.

**Skill existence is not among the causes.** With the gate open and a
non-view-scoped key, the assignment path creates the agent-skill payload
row *lazily*, so an environment holding zero skills is served an empty
payload that commits normally through `payload-transferred` and
initializes the store. The old message claimed the condition was about
whether any skill existed, and promised a skill created later would
arrive without a restart. Both were false; neither is said now. This
does not assume gonfalon #73018 (eager row creation) lands — it is
closed.

## What changed

**`packages/client/src/skills-fdv2.ts`**

- `classifyStatus` returns a `FatalTransportError` for 422, in **its own
branch** above the generic fatal list so the message can be specific.
The message is a contract — it is what a customer pastes into a support
ticket — so it names both real causes, but **weights them**: the
view-scoped SDK key leads and carries the instruction, because it is the
only one of the two a reader can act on. Skill delivery is not enabled
per account as a customer-facing step, so a closed gate is a
LaunchDarkly-side condition (a kill switch, or a rollout that has not
reached them); instructing someone to go enable Agent Skills for their
account would send them looking for a setting they do not have, so that
cause routes to support instead. Both docs carry the same weighting, and
the tests assert it — including negative matches against the "enable it
for your account" phrasing.
- Deleted `NoSkillPayloadError` and its doc comment. It was `export`ed
from the module but not from the package index, so `src/index.ts` needed
no change; the test file was the only other importer.
- Deleted both run-loop branches (the `payloadUnavailable` increment and
the retry-at-cap ternary), plus the once-per-process "delivery is idle"
warning flag. A fatal reaches the give-up path through existing fatal
handling with no new code, and the delay expression is now just the
ordinary backoff path.
- Removed `payloadUnavailable` from the `StoreDiagnostics` declaration
and from `freshDiagnostics()`. The declaration is the part the spec's
absence assertion is about: a field still declared but never incremented
still fails it.

**Accounting comes out right for free.** A fatal 422 sets `failed` and
`lastError` and does **not** touch `connectionFailures` — that counter
measures consecutive *recoverable* failures against the retry bound, and
a fatal never retries, so moving it would put a number against a budget
nothing will spend and make a store that gave up on its first response
look like one that exhausted its attempts. This is already how `giveUp`
accounts 401 and 404, so routing 422 through it needed no code; the
tests assert it rather than adding any. `giveUp` also already calls
`releaseWaiters()`, which is what makes `waitForSkills` resolve `false`
immediately.

**Tests** (`packages/client/src/__tests__/skills-fdv2.test.ts`)

Removed the `NoSkillPayloadError` import and every
`diagnostics.payloadUnavailable` assertion; the `waitUntil(...)` helpers
that polled that counter now poll `store.failed`. Deleted the
retry-at-cap case (there is no retry to schedule) and the
skill-arrives-after-422 case (it asserted behavior the spec now forbids
promising). Inverted the keeps-delivering case: a 422 on the **first**
response stops delivery and sets `failed`, asserted against a
`maxConsecutiveFailures` of 10 so what stops the loop is provably the
classification and not an exhausted budget.

Added, per the spec:

- `waitForSkills` resolves `false` **immediately** — value *and* timing,
against a 10s timeout that waiting out would be plainly visible,
measured with `performance.now()` (the suite's existing monotonic clock)
rather than wall-clock sleeps.
- The message names the account-level enablement and the key's scoping,
asserted on substance so the wording can be improved without rotting the
test — plus negative matches for the two claims it must not make, and
for telling the reader to enable Agent Skills for their own account.
- `connectionFailures` unmoved after a fatal 422, while `failed` and
`lastError` are set.
- `payloadUnavailable` **absent** from `StoreDiagnostics`, asserted both
ways: by source text (matching the `FDV2_OBJECT_KIND` precedent, and
also catching the deleted warning flag and idle-warning string) and as a
missing key on a real diagnostics snapshot. A type-level removal is
invisible at runtime, and a source check would pass a field added
dynamically — neither alone is the whole fact.
- Classification answers every status with exactly one of the two
classes, swept over 29 statuses. The spec forbids the *shape*, not just
the name: a status handled as neither recoverable nor fatal is a retry
loop with no bound and no budget, invisible to `failed` and
`connectionFailures` alike.

The existing 422 cases drive the real loopback `node:http` fake
endpoint, and the rewrites keep that pattern per A.12's TypeScript
exception.

**Docs.** `packages/client/agents.md` and `packages/client/README.md`
each carried a paragraph documenting the deleted class, counter, and
retry-at-cap as designed behavior, and the README listed
`payloadUnavailable` among the `StoreDiagnostics` fields. Rewritten to
the fatality, the platform reasoning, the two real causes, and the
accounting. Slightly beyond the strict file scope, but leaving them
would ship documentation for a deleted class.

## The one cost, documented so it is not rediscovered as a bug

A process whose store gave up needs a restart once the cause is resolved
— `close()` is final and nothing reopens delivery short of constructing
a new store. That is deliberately preferred to a store that retries a
permanent rejection for the life of the process. Stated in both docs
rather than left to be found.

## Note for the Python side

The message wording was going to be kept aligned with the Python SDK's,
and this re-weighting moves it — so the parallel Python change wants the
same treatment: lead with the view-scoped key, route the account gate to
support, and drop any instruction to go enable Agent Skills for the
account. The spec's requirement that both causes be named is still met
on both sides.

## Scope

The `?kinds=agent-skill` declaration, the payload/object kind constants,
and the rest of the transport are untouched. `xie/skills-kinds-param`
was not rebased or amended.

## Verification

All pass, nothing worked around:

- `@launchdarkly/ai-server` tests — **1028 passed, 10 skipped** (the
skips are pre-existing)
- `tsc --noEmit` for the client, and `yarn typecheck` across all
workspaces
- `biome check` clean; lefthook's `lint` and `code-check` passed on
commit

🤖 Generated with [Claude Code](https://claude.com/claude-code)
XieX added a commit that referenced this pull request Sep 29, 2026
Stacked on #87 — base is `xie/skills-kinds-param`, not `main`.

Declaring `?kinds=agent-skill` changed what arrives on the connection,
and four places still described the pre-declaration shape. This is the
JS counterpart of launchdarkly/python-ai-sdk#109 (`91c9e20`).

### README

- **"The connection also carries your flags"** told customers a client
cannot request only the skill payload, and that `objectsIgnored` counts
the flag and segment objects arriving anyway. Both halves are now the
opposite of what happens: `FDV2_PAYLOAD_KIND` is on every request, which
is what makes the connection carry exactly one payload.
- the **`addListener`** paragraph repeated the same claim in passing, as
the reason a non-skill kind is never dispatched.

`objectsIgnored` keeps an explanation rather than losing one, because
the counter still exists and now means something narrower and worth
saying: an object kind this version does not recognise, not the
environment's flags. That wording matches Python's `objects_ignored`,
which rewrote this paragraph rather than deleting it.

### Source and agents.md

- **`objectsIgnored`'s own doc comment** said what the README paragraph
did, so the public API docs still told a reader the counter tallies
their flags.
- **`REQUEST_ADVICE`** enumerates what the request carries and left
`kinds` out. It is the text a user reads on the 400/405/406/414/501
family — exactly what an endpoint that does not understand `kinds` would
answer — so the omission pointed at the base URI instead of the likely
cause.
- **agents.md** attributed single-payload delivery to delivery itself
("Delivery provides one payload per credential"), contradicting the
paragraph twelve lines above: a skill-enabled environment assigns two,
and it is the declaration that narrows the connection to one.

### Deliberately unchanged

- `payloadUnavailable` documents itself with `{@link
NoSkillPayloadError}`, which is fine here because that class is
exported; Python's equivalent was private, which is why it was reworded
there.
- `payloadsIgnored` keeps "Zero while delivery sends one payload per
connection" — the declaration makes that condition unconditional rather
than false, and Python left its wording alone too.

### Known divergence

Python's agents.md still carries the "one payload per credential" claim
fixed here, so the two trees differ on that line until python-ai-sdk
catches up.

### Gate

`tsc --noEmit` clean, `biome check` clean, `vitest run` 1024 passed / 10
skipped in `packages/client` — the same counts #87 recorded, and the ten
are the capability-gated TOCTOU tests. Docs only; no behaviour change,
so no tests added.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
XieX added a commit to launchdarkly/python-ai-sdk that referenced this pull request Sep 29, 2026
…ls yet" (#109)

Stacks on #94 -> #93 -> #87. Spec:
[ai-sdks-monorepo#23](launchdarkly/ai-sdks-monorepo#23).
TypeScript counterpart:
[js-ai-sdk#87](launchdarkly/js-ai-sdk#87).
Server side:
[streamer#4730](launchdarkly/streamer#4730).

This is the SDK half of the release blocker: other SDKs retry forever
when they see a skill payload. Flag Delivery's answer is `?kinds=`,
which narrows a connection to the payload kinds it declares, defaulting
to `{flagging}` (so we need to opt-in to see skills).

## The declaration

Every request now carries `kinds=agent-skill` — both endpoints, and on
the first request as well as the ones after it, since it selects what
the connection is served rather than describing what the store already
holds. Without it the store receives the environment's flag payload and
no skills at all, so this is a functional requirement, not for
correctness/optimization.

It also fixes something that was already wrong. A skill-enabled
environment assigns two payloads, so `_ProtocolReader` has been warning
about the second on every connection, reading only the first intent, and
never adopting a basis for the flag payload — re-downloading and
discarding it on every reconnect. Declaring one kind makes the
connection single-payload, which is the shape the reader is built for.
(That is also why declaring `flagging,agent-skill` is not the
safe-looking option it appears to be.)

No `mv`, still — but for a corrected reason. It selects the *flag* data
model, and `objectQueryForCommand` overrides whatever a request asks for
with the payload's own default for any non-flagging payload, so sending
it would state a preference that is ignored. The old comment said the
connection would be refused over it.


🤖 Generated with [Claude Code](https://claude.com/claude-code), edited
by @XieX



<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> **FDv2 skill delivery** now sends `kinds=agent-skill` on every
poll/stream request (with `basis` when applicable), so connections
receive only the agent-skill payload instead of flags plus skills.
**`HTTP 422` is treated as a fatal, non-retryable error** (aligned with
the TypeScript SDK): delivery stops, `failed`/`last_error` are set,
`connection_failures` is unchanged, `wait_for_skills` returns
immediately, and operators are pointed at **`start()` on the same
store** after fixing the cause (e.g. view-scoped SDK key) rather than
only a process restart.
> 
> **Integrity logging** gains a tenth `reason_code`,
**`version_mismatch`**, via `record_version_mismatch` when a pinned
version does not match what the store returned (`served_version` on the
log record; **`wrong_version`** on `get_skill_result`). Like
**`key_mismatch`**, it emits the **`ld.skills.integrity_failure` log
only**—no product integrity signal. README/agents.md and tests cover
422, `kinds`, give-up messaging, and the write path for version
mismatches.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
260856b. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@XieX XieX changed the title feat(client): declare ?kinds=agent-skill, and treat a 422 as "no skills yet" feat(client): declare ?kinds=agent-skill, and make a 422 a fatal transport failure Sep 30, 2026
@XieX
XieX changed the base branch from xie/skills-14-review-closeout to xie/skills-14d-robustness September 30, 2026 16:58
XieX and others added 13 commits September 30, 2026 12:59
…ls yet"

Delivery now narrows a connection to the payload kinds it declares and defaults
to flags (launchdarkly/streamer#4730), so the store has to ask for the
agent-skill payload or receive the environment's flags and no skills at all.
The declaration goes on every request, before any basis exists as well as
alongside one: it selects what the connection is served rather than describing
what the store already holds.

It also fixes something that was already wrong. A skill-enabled environment
assigns two payloads, so the reader has been warning about the second and
reading only the first intent, and the flag payload was re-downloaded and
discarded on every reconnect because its basis was never adopted. Declaring one
kind makes the connection single-payload, which is the shape the reader is
built for.

The 422 that comes with it is the interesting half. It is the answer when the
credential is assigned no agent-skill payload, which is every project where no
skill has ever been created -- gonfalon creates that row with the first skill
and never lazily. As an ordinary recoverable failure it would spend
maxConsecutiveFailures and then report "gave up after N consecutive failures:
LaunchDarkly returned HTTP 422" for an ordinary configuration; as a fatal one,
the skill created a minute later would never arrive without a process restart.
So it is its own class: NoSkillPayloadError, which reuses the existing
`expected` flag to stay off connectionFailures, lastError, failed and the
per-attempt warning, is said once per store, counted under the new
payloadUnavailable diagnostic, and retried at maxBackoffMs indefinitely. The
retry is at the cap because `failures` deliberately never moves, so the
exponential schedule would otherwise sit at the initial delay forever.

Seven tests, each of which fails with the source reverted: the declaration on
both endpoints and on both hosts, the two kind constants held apart by source
text, the 422's classification, that it never stops delivery and never counts,
that it waits the cap and not the initial delay, that a skill arriving after it
is picked up, and that it leaves the store uninitialized so a wildcard
reconcile prunes nothing. The fake endpoint gained a standing default status,
since "every request is answered 422" is not something a queue can express.

Also corrects docs that described the payload as classified `generic`, and the
`mv` rationale: delivery overrides the requested model version for any
non-flagging payload rather than refusing the connection over it.

Gate: tsc --noEmit clean, biome check clean, vitest run 1024 passed / 10
skipped in packages/client (the ten are the capability-gated TOCTOU tests),
and every workspace's suite green from the root.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirrors the Python trim (launchdarkly/python-ai-sdk#109): the same sentence had
been repeated at every site that touches the 422, so NoSkillPayloadError now
owns the explanation and the others state only what is local to them — the
diagnostic links the type and keeps the "not a connectionFailures" distinction,
the loop keeps why it waits the cap, and classifyStatus keeps nothing, since
the type it returns and the message it builds already say it twice over.

No behaviour change, and the user-facing 422 message is untouched. 12 lines of
comment removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…payload

Not this PR's change -- the case is byte-identical to the one on the base
branch -- but it went red on this PR's CI run and will keep doing so on a
loaded runner.

It queued the good payload and the hashless one up front, 20ms apart, then
relied on `waitForSkills` before reconciling. `waitForSkills` promises the
*first* commit and nothing about the second, so any pause longer than the poll
interval between it and the first `writeSkills` -- an `mkdtemp` on a busy
runner is enough -- lets the hashless payload commit first. Verification then
withholds every object, the reconcile writes nothing, and the assertion fails
as `ENOENT` on the read rather than as anything that names the cause.

Queueing the hashless payload only after the first reconcile has been asserted
removes the race without changing what the case asserts. Reproduced before the
fix by standing a 60ms sleep in for the runner (identical ENOENT), and the
fixed case survives 300ms in the same spot.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LaunchDarkly answers 422 when a connection's declared kinds exclude every
payload it is assigned, and it picked a non-400 4xx *because* LD SDKs treat
those as terminal — the streamer says so in as many words at both places that
emit it, each noting the code exists so a misconfigured SDK stops instead of
hammering the fleet. Retrying it forever was the precise behavior the status
was chosen to prevent.

The stated cause was also wrong. The gate is skill delivery enabled for the
account *and* a credential that is not view-scoped; with both satisfied the
assignment path creates the agent-skill payload row lazily, so an environment
holding zero skills is served an empty payload that commits normally. Skill
existence is not among the causes, so the message no longer claims it is, and
no longer promises that a skill created later arrives without a restart.

`classifyStatus` gets its own 422 branch rather than folding into the generic
fatal list, because the message is a contract — it is what a customer pastes
into a support ticket. It names both causes but weights them: the view-scoped
SDK key leads and carries the instruction, because it is the only one of the
two a reader can act on. Skill delivery is not enabled per account as a
customer-facing step, so a closed gate is a LaunchDarkly-side condition — a
kill switch, or a rollout that has not reached them — and telling someone to
go enable Agent Skills for their account would send them looking for a setting
they do not have. That cause routes to support instead.

Routing 422 through the existing give-up path gives the right accounting for
free: `failed` and `lastError` are set, `connectionFailures` is untouched (it
measures consecutive *recoverable* failures against the retry bound, which a
fatal never spends), and `waitForSkills` resolves `false` at once instead of at
the caller's timeout.

`NoSkillPayloadError` and `diagnostics.payloadUnavailable` are deleted rather
than merely unexported. A status handled as neither recoverable nor fatal is a
retry loop with no bound and no budget, invisible to `failed` and
`connectionFailures` alike, so the shape is forbidden and not just the name:
tests assert that classification answers every status with exactly one of the
two classes, and assert both absences by source text and on a real
diagnostics snapshot.

The cost, stated in the docs so it is not rediscovered as a bug: a process
whose store gave up needs a restart once the cause is resolved, since
`close()` is final and nothing reopens delivery short of a new store. That is
deliberately preferred to retrying a permanent rejection for the life of the
process.

Conforms to ai-sdks-monorepo #29 (TESTING.md §3.25, A.12).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Declaring `?kinds=agent-skill` changed what arrives on the connection, and
two paragraphs in the client README still described the old shape:

- "The connection also carries your flags" told customers a client cannot
  request only the skill payload and that `objectsIgnored` counts the flag
  and segment objects that arrive anyway. Both halves are now the opposite
  of what happens: `FDV2_PAYLOAD_KIND` is on every request, which is what
  makes the connection carry exactly one payload.
- the `addListener` paragraph repeated the same claim in passing, as the
  reason a non-skill kind is never dispatched.

`objectsIgnored` keeps an explanation rather than losing one, because the
counter still exists and now means something narrower and worth saying: an
object kind this version does not recognise, not the environment's flags.
That matches the Python SDK's wording for `objects_ignored`
(python-ai-sdk#109, 91c9e20), which rewrote this paragraph rather than
deleting it; the two READMEs stay in parity.

`payloadsIgnored` needed no change here. The README table lists it without
prose, so the "Zero while delivery sends one payload per connection"
wording is only on the field's own doc comment in skills-fdv2.ts, and the
declaration makes that condition unconditional rather than false.

README only. The stale `objectsIgnored` doc comment in skills-fdv2.ts, and
the same claim in agents.md, are untouched and still to do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README fix left three places still describing the pre-declaration shape.

`objectsIgnored`'s own doc comment said what the README paragraph did —
"flags, segments, and any future kind" — so the public API docs still told
a reader the counter tallies their environment's flags. It now says what it
actually counts, matching the README and the Python docstring.

`REQUEST_ADVICE` enumerates what the request carries and left `kinds` out.
It is the text a user reads on the 400/405/406/414/501 family, which is
exactly what an endpoint that does not understand `kinds` would answer, so
the omission pointed at the base URI instead of the likely cause.

agents.md attributed single-payload delivery to delivery itself ("Delivery
provides one payload per credential"), which the paragraph twelve lines
above now contradicts: a skill-enabled environment assigns two, and it is
the declaration that narrows the connection to one. The reason is now the
declaration, which is also what makes the rest of that paragraph — the
reader's payload-id check and `payloadsIgnored` — read as the residual it
is rather than as a guarantee.

Two things Python changed that JS deliberately does not. `payloadUnavailable`
documents itself with `{@link NoSkillPayloadError}`, which is fine here
because that class is exported; the Python equivalent was private, which is
why it was reworded there. And `payloadsIgnored` keeps "Zero while delivery
sends one payload per connection": the declaration makes that condition
unconditional rather than false, and Python left its wording alone too.

Note that Python's agents.md still carries the "one payload per credential"
claim this commit fixes, so the two trees diverge on that line until
python-ai-sdk catches up.

Gate: tsc --noEmit clean, biome check clean, vitest run 1024 passed / 10
skipped in packages/client — the same counts the declaration commit
recorded, and the ten are the capability-gated TOCTOU tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`resolveFromStore` withholds a skill when a pin was answered with a
different version, and wrote nothing at all: no log record, no signal,
and no stated reason for the silence in the code, in `agents.md`, or in
TESTING.md. The branch immediately above it — `key_mismatch` — carries
fourteen lines explaining which surfaces fire and why.

`recordVersionMismatch` fills the gap, following 0f90767's shape:

- log record only, `reason_code: version_mismatch`, no product signal.
  The usual cause of either boundary check firing is a broken custom
  store adapter, and LaunchDarkly's own counter must not fill with
  customers' adapter bugs. Pinned in both directions, because an
  implementation that emitted the signal too would look correct from
  every other angle;
- the record carries `version` (the version **requested**, the meaning
  it has on every other record) and a record-only `served_version`
  paralleling `served_key`. No hash fields: verification passed. Keys
  inserted alphabetically so `JSON.stringify` stays byte-identical to
  Python's `json.dumps(record, sort_keys=True, separators=(",",":"))`,
  modulo `language`;
- `IntegrityReasonCode` grows from nine tokens to ten.
  `SkillOutcomeReason` stays a closed five — this changes the detection
  surface, not the public outcome vocabulary.

The code is spelled `version_mismatch` rather than `wrong_version` to
keep the `*_mismatch` family consistent and to keep one string out of
two closed vocabularies with two meanings.

Worth recording even though `wrong_version` is already a public outcome
token, and even though neither shipped store can reach the branch:
`getSkill` is the documented default and collapses `wrong_version` to
`null` exactly as it collapses `integrity_failure`, so without the
record an operator on the default accessor has no visibility into a
store answering pins with the wrong version. Tests therefore assert it
through `getSkill` as well as `getSkillResult`, and use a hand-built
store that ignores its version argument — a test built on a shipped
store gets `absent` and silently stops covering the case.

Also pinned: the key check wins when a store disagrees on both key and
version (it decides the caller-visible outcome, not just the code), the
two versions are recorded as integers rather than strings, and the
listing path reports no mismatch for a store spelling its map keys
`key:version`, mirroring 4cfb753's guard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`1aa1594` corrected `objectsIgnored` and `REQUEST_ADVICE`; two doc
comments still give the old shape as the *reason* for a behaviour rather
than merely describing it:

- `isSkillEvent` says a foreign kind is ignored rather than rejected
  "because flag and segment objects share the connection". With the
  declaration in place they do not. The skip is still right and still
  tested, but the reason is now forward compatibility: erroring on an
  unrecognised kind would turn a payload that gained one into the
  reconnect-loop outage this feature must not cause, which is how
  `agents.md` already puts it;
- `addListener`'s doc comment is the twin of the README paragraph
  adc270f fixed, and repeated the same claim in passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Python side of this change landed on `xie/agent-skills`
(python-ai-sdk#87), so the parity claim is now checkable by running both
recorders rather than by reading one and reasoning about the other. Doing
that turned up one divergence on the same input.

Python routes `served_version` through `is_valid_skill_version` and
records `<invalid-version>` when it fails; this side recorded the value
verbatim on the strength of its `number` type. For every reachable input
the two agree — `verify_raw_skill` / `verifyRawSkill` has already
accepted the served version by the time either recorder runs — so the
divergence only shows on the path neither SDK can reach today. It is
still worth closing, for the two reasons `served_key`'s identical guard
already exists on this side: unreachability is a property of the current
call order rather than of the recorder, and the check is what keeps the
field an integer, which the byte comparison depends on (`3` and `"3"`
are not the same line).

`requested` stays unchecked in both languages, and that asymmetry is
deliberate: it is the caller's own pin rather than a store-controlled
value, so it cannot carry skill content, and coercing a mistyped one
would hide the caller's mistake from their own log.

Verified by executing both implementations over three inputs — the
reachable case and each guard — and diffing: byte-identical modulo
`language`. The README's `served_version` row now documents the
placeholder, as Python's does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Infinity` passed the guard, because the check was `Number.isNaN(timeout) ||
timeout < 0` and `Infinity` is neither. It then made `deadline` non-finite,
which voided the bound the option documents — "`timeout` bounds the whole call,
including content retrieval" — and let the call run unbounded instead.

The guard is finiteness now, which covers `Infinity` and `NaN` together and
matches the three sibling guards that already read that way: `debounceMs` in
skills-watch, and `pollIntervalMs` / `readTimeoutMs` / `waitForSkills` in the
FDv2 store. agents.md already claimed `timeout` was "guarded for sign and
finiteness"; this makes that true rather than aspirational.

`NaN` is worth naming separately even though it was already rejected here,
because its failure mode depends on the shape of the comparison rather than on
the value: `now > deadline` reads as never expiring and `deadline - now > 0`
reads as already expired, so one input can push two implementations in opposite
directions with neither detectably wrong from inside its own language. That is
why the cross-SDK spec states this as finiteness and not as a `NaN` footnote.

The message reports the value with `String` rather than `JSON.stringify`, which
renders both `NaN` and `Infinity` as `null` and so named a mistake the caller
did not make. Same reasoning, and same wording, as the `debounceMs` guard.

Spec: launchdarkly/ai-sdks-monorepo#32 (§3.22, §3.26).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@XieX
XieX force-pushed the xie/skills-kinds-param branch from e890a0f to 9928c9d Compare September 30, 2026 17:08

@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 9928c9d. Configure here.

Comment thread packages/client/src/skills-fdv2.ts
@XieX
XieX merged commit 9151d4a into xie/skills-14d-robustness Oct 1, 2026
8 checks passed
@XieX
XieX deleted the xie/skills-kinds-param branch October 1, 2026 19:06
@jeffdupont jeffdupont mentioned this pull request Oct 2, 2026
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