Skip to content

feat(client): declare ?kinds=agent-skill, and treat a 422 as "no skills yet" - #109

Open
XieX wants to merge 4 commits into
xie/agent-skillsfrom
xie/python-skills-kinds-param
Open

XieX wants to merge 4 commits into
xie/agent-skillsfrom
xie/python-skills-kinds-param

Conversation

@XieX

@XieX XieX commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Stacks on #94 -> #93 -> #87. Spec: ai-sdks-monorepo#23. TypeScript counterpart: js-ai-sdk#87. 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} (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.

The 422

Delivery answers 422 no_accepted_payloads when the credential's assignment holds no payload of the declared kind. That is every project in which no skill has ever been created: gonfalon's payloadIDsForEnvironment appends the agent-skill payload ID only "if one already exists" and never lazily creates it.

Both obvious classifications are wrong. Reverting just the 422 case and rerunning the new tests prints exactly what the recoverable reading gives you:

Skill delivery has stopped and will not retry: gave up after 2 consecutive
failures; last error: LaunchDarkly returned HTTP 422.

...for an environment whose only problem is that nobody has made a skill yet. Fatal is no better: the skill created a minute later never arrives until the process restarts.

So _NoSkillPayloadError, caught ahead of _RecoverableTransportError (which it subclasses, so nothing else has to change):

  • counted under the new StoreDiagnostics.payload_unavailable;
  • logged once per store, with a message that names the state rather than the status;
  • retried at max_backoff indefinitely — at the cap because _failures deliberately never moves, so the exponential schedule would otherwise sit at the initial delay forever;
  • kept off connection_failures, last_error and failed.

It commits no payload, so is_initialized() stays false and write_skills("*") still withholds the prune. That is the right answer: "LaunchDarkly has no skill payload for this environment" and "this environment's every skill was revoked" are the two readings of an empty answer, and only the second may delete a customer's files.

Tests

Nine, each of which fails with the source reverted:

  • the declaration on /sdk/poll (first request and the one carrying a basis) and on /sdk/stream, and on the from-scratch 400 retry — where the two existing query == {} assertions became {"kinds": ...}, which is a better assertion: the declaration is not client state;
  • FDV2_PAYLOAD_KIND and FDV2_OBJECT_KIND held apart;
  • the 422's classification, and that its message explains the state;
  • that a 422 repeated well past max_consecutive_failures=1 neither stops delivery nor counts, and is said exactly once;
  • that it waits the cap and not the initial delay (recording _stop so the assertion is on the interval asked for, not on wall-clock timing);
  • that a skill payload arriving after two 422s is picked up, with both diagnostics reading as what happened;
  • that it leaves the store uninitialised, so a wildcard reconcile leaves a stale file on disk.

The fake endpoint gained default_poll_status, since "every request is answered 422" is not something a queue can express.

Gate

From python/: make test (1888 passed, 11 skipped), make typecheck (clean, 51 source files), make lint, make format-check — all clean.

🤖 Generated with Claude Code, edited by @XieX


Note

Overview
FDv2 skill delivery now opts into the agent-skill payload on every poll/stream request via ?kinds=agent-skill, replacing the previous default flag payload. That makes connections single-payload (matching _ProtocolReader) and is required for skills to arrive at all; basis retries still send kinds because it is not client state.

HTTP 422 (“no payload of this kind”) is treated as an idle waiting state, not a transport failure: new _NoSkillPayloadError is handled before the recoverable path, increments StoreDiagnostics.payload_unavailable, logs once, retries at max_backoff without touching failed/connection_failures/last_error, and leaves is_initialized() false so write_skills("*") does not prune. Docs and tests cover declaration on poll/stream, 422 classification, backoff cap, pickup after first skill is created, and prune safety.

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

…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
max_consecutive_failures and then report "gave up after N consecutive failures:
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, caught ahead of _RecoverableTransportError, said
once, counted under the new payload_unavailable diagnostic, retried at
max_backoff indefinitely, and kept off connection_failures, last_error and
failed. The retry is at the cap because _failures deliberately never moves, so
the exponential schedule would otherwise sit at the initial delay forever.

Nine tests, each of which fails with the source reverted: the declaration on
both endpoints and on a from-scratch retry, the two kind constants held apart,
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 uninitialised 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
request as carrying `mv`.

Gate from python/: make test (1888 passed, 11 skipped), typecheck, lint,
format-check all clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"The answer for a project in which no skill has ever been created" appeared
seven times in one file: on the diagnostic, on the exception class, in
_classify_status, on the once-per-store flag, and twice in the delivery loop.

_NoSkillPayloadError now owns the explanation, since it is what the other sites
refer to, and each of those states only what is local to it: the diagnostic
names the type and keeps the "not a connection_failures" distinction, the
except block keeps why it is caught first and why it waits the cap, and
_classify_status keeps nothing -- 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. 15 lines of
comment removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
XieX added a commit to launchdarkly/js-ai-sdk that referenced this pull request Sep 24, 2026
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>
Base automatically changed from xie/python-skills-revoke-by-omission to xie/python-agent-skills-review-fixes September 24, 2026 19:33
Base automatically changed from xie/python-agent-skills-review-fixes to xie/agent-skills September 24, 2026 19:33
XieX and others added 2 commits September 28, 2026 12:15
… the 422

The declaration changed what arrives on the connection, and three places
still described the old shape:

- the README told customers the connection also carries their flags and
  that `objects_ignored` counts them, which is now the opposite of what
  happens;
- `objects_ignored`'s own docstring said the same;
- `_REQUEST_ADVICE` enumerates what the request carries, and 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 leaving the parameter
  out of it pointed at the base URI instead of the likely cause.

Also, on the new text: the 422 message and the log line it lands in read as
one 70-word paragraph that stated the retry cadence twice, so the cadence is
now the log's alone; `payload_unavailable` is a public field and documented
itself with a private class name, so it names the status instead; and the
internal service name behind "created with the first skill" is out.

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

Two small things in the README's store section. "`connection_failures` stays
at zero" is only true of a store that has had no other trouble; what the
handler guarantees is that the 422s are kept off that counter and off
`last_error`, so say that instead. And the `StoreDiagnostics` field list
omitted `payloads_ignored`, which the type has carried all along.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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