feat(client): declare ?kinds=agent-skill, and make a 422 a fatal transport failure - #87
Merged
Merged
Conversation
This was referenced Sep 29, 2026
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 -->
knfreemLD
approved these changes
Sep 30, 2026
XieX
changed the base branch from
xie/skills-14-review-closeout
to
xie/skills-14d-robustness
September 30, 2026 16:58
…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
force-pushed
the
xie/skills-kinds-param
branch
from
September 30, 2026 17:08
e890a0f to
9928c9d
Compare
There was a problem hiding this comment.
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 9928c9d. Configure here.
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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
ProtocolReaderhas been hittingwarnMultiplePayloadson 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 declaringflagging,agent-skillis not the safe-looking option it appears to be.)No
mv, still — but for a corrected reason. It selects the flag data model, andobjectQueryForCommandoverrides 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-skillon 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/lastErrorare set,connectionFailuresstays at zero,waitForSkillsresolvesfalseimmediately, and an uninitialized store still blocks wildcard reconcile from pruning disk.Observability: pinned-version mismatches emit a new SIEM record with
reason_code: version_mismatchandserved_version(log only, no product signal), parallel tokey_mismatch; docs and tests expand the integrity vocabulary from nine to ten tokens.writeSkillsrejects non-finitetimeoutvalues (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.