Skip to content

fix(client): make HTTP 422 a fatal transport failure - #93

Merged
XieX merged 3 commits into
xie/skills-kinds-paramfrom
xie/fatal-422-js
Sep 29, 2026
Merged

XieX merged 3 commits into
xie/skills-kinds-paramfrom
xie/fatal-422-js

Conversation

@XieX

@XieX XieX commented Sep 29, 2026 •

Copy link
Copy Markdown

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 exported 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


Note

Overview
FDv2SkillStore now treats HTTP 422 as a fatal transport error instead of an indefinite “no payload yet” retry. classifyStatus(422) returns FatalTransportError with messaging aimed at view-scoped SDK keys and account enablement—not “create your first skill.” Delivery stops on the first 422: failed / lastError are set, waitForSkills resolves false immediately, and connectionFailures is unchanged.

The NoSkillPayloadError type, payloadUnavailable on StoreDiagnostics, the idle-delivery warning, and retry-at-maxBackoffMs special casing are removed. Tests and README / agents.md are updated to match platform semantics (422 is terminal; empty environments still get a normal empty payload when delivery is allowed).

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

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>
@XieX
XieX marked this pull request as ready for review September 29, 2026 17:01
@XieX
XieX merged commit b9be695 into xie/skills-kinds-param Sep 29, 2026
5 of 6 checks passed
@XieX
XieX deleted the xie/fatal-422-js branch September 29, 2026 17:02
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