fix(client): make HTTP 422 a fatal transport failure - #93
Merged
Merged
Conversation
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
force-pushed
the
xie/fatal-422-js
branch
from
September 29, 2026 16:28
a686326 to
d4d7b77
Compare
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.
Stacked on #87 (
xie/skills-kinds-param), which is where the?kinds=agent-skillwork 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
streamerrepo 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.SkillDeliveryAllowedin 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-transferredand 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.tsclassifyStatusreturns aFatalTransportErrorfor 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.NoSkillPayloadErrorand its doc comment. It wasexported from the module but not from the package index, sosrc/index.tsneeded no change; the test file was the only other importer.payloadUnavailableincrement 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.payloadUnavailablefrom theStoreDiagnosticsdeclaration and fromfreshDiagnostics(). 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
failedandlastErrorand does not touchconnectionFailures— 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 howgiveUpaccounts 401 and 404, so routing 422 through it needed no code; the tests assert it rather than adding any.giveUpalso already callsreleaseWaiters(), which is what makeswaitForSkillsresolvefalseimmediately.Tests (
packages/client/src/__tests__/skills-fdv2.test.ts)Removed the
NoSkillPayloadErrorimport and everydiagnostics.payloadUnavailableassertion; thewaitUntil(...)helpers that polled that counter now pollstore.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 setsfailed, asserted against amaxConsecutiveFailuresof 10 so what stops the loop is provably the classification and not an exhausted budget.Added, per the spec:
waitForSkillsresolvesfalseimmediately — value and timing, against a 10s timeout that waiting out would be plainly visible, measured withperformance.now()(the suite's existing monotonic clock) rather than wall-clock sleeps.connectionFailuresunmoved after a fatal 422, whilefailedandlastErrorare set.payloadUnavailableabsent fromStoreDiagnostics, asserted both ways: by source text (matching theFDV2_OBJECT_KINDprecedent, 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.failedandconnectionFailuresalike.The existing 422 cases drive the real loopback
node:httpfake endpoint, and the rewrites keep that pattern per A.12's TypeScript exception.Docs.
packages/client/agents.mdandpackages/client/README.mdeach carried a paragraph documenting the deleted class, counter, and retry-at-cap as designed behavior, and the README listedpayloadUnavailableamong theStoreDiagnosticsfields. 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-skilldeclaration, the payload/object kind constants, and the rest of the transport are untouched.xie/skills-kinds-paramwas not rebased or amended.Verification
All pass, nothing worked around:
@launchdarkly/ai-servertests — 1028 passed, 10 skipped (the skips are pre-existing)tsc --noEmitfor the client, andyarn typecheckacross all workspacesbiome checkclean; lefthook'slintandcode-checkpassed on commit🤖 Generated with Claude Code
Note
Overview
FDv2SkillStorenow treats HTTP 422 as a fatal transport error instead of an indefinite “no payload yet” retry.classifyStatus(422)returnsFatalTransportErrorwith messaging aimed at view-scoped SDK keys and account enablement—not “create your first skill.” Delivery stops on the first 422:failed/lastErrorare set,waitForSkillsresolvesfalseimmediately, andconnectionFailuresis unchanged.The
NoSkillPayloadErrortype,payloadUnavailableonStoreDiagnostics, the idle-delivery warning, and retry-at-maxBackoffMsspecial 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.