fix(skills): apply changes after a none intent, and retry recoverable failures indefinitely

[XieX]

Addresses three review comments on #87. Targets xie/agent-skills.

Changes

• Apply objects that follow a none intent (r4167784120). After none, the reader now expects changes, as the base SDK's ChangeSetBuilder.expect_changes() does. Before, a put-object / delete-object after none on the same stream was dropped while the following payload-transferred still advanced the basis. A skill revoked after a routine reconnect kept being served, and reconnecting didn't fix it.
• Remove max_consecutive_failures; retry recoverable failures indefinitely (r4167784131). Only fatal statuses (401/403/404/422 etc., a second 400) stop delivery and set failed. The 400-retried-once repair is unchanged.
• Clamp the backoff exponent. float(2 ** n) in _backoff_delay raises OverflowError past ~1024 consecutive failures. The old budget kept the count from getting there; with unlimited retries a long outage (~8.5 h at the default cap) would have crashed the delivery thread.
• Scope the README revocation claim to "*" (r4167784141, docs only). With an explicit list, an absent skill stays requested with an error action and isn't pruned, and the watcher doesn't see config changes. Also fixes the _resolve_requests docstring, which said absent makes a run incomplete.

Tests

• New: put after none, delete after none (reader level), and a revocation after a reconnect answered none (end to end on the stream). All three fail with the fix reverted.
• Budget tests rewritten: retried well past 10 failures (poll and stream) with last known good still served; an announced-then-dropped transfer counts each drop; restart and commit reset the count; max_consecutive_failures is absent by name; backoff at attempt 10,000 is finite. Tests that used a small budget only to stop the store now use a fatal status. Two tests about the budget's 400 exemption were deleted.
• make test (2166 passed, 11 skipped), make lint, make format-check, make typecheck all pass. Ran the affected tests 10x with no flakes.

The same changes for JS: launchdarkly/js-ai-sdk#108.
Spec: launchdarkly/ai-sdks-monorepo#36

🤖 Generated with Claude Code

---

Note

Overview
FDv2SkillStore delivery behavior is tightened so revocations and outages match the base SDKs and review feedback on #87.

After a server-intent with none, the protocol reader now expects later put-object / delete-object events on the same stream (aligned with ChangeSetBuilder.expect_changes()). Previously those edits could be ignored while payload-transferred still advanced the basis, so a skill revoked right after a routine reconnect could keep being served.

Recoverable transport failures retry for the life of the store — max_consecutive_failures is removed from the public API. Only fatal outcomes (401/403/404/422, a second 400, oversize bodies/events via new _ResponseTooLargeError, etc.) set failed and stop delivery; start() clears terminal state and resets backoff. Backoff uses a separate attempt counter (reset after a completed poll or a stream held ≥60s), routine post-answer goodbye disconnects are marked recycled and not counted as failures, and the backoff exponent is clamped so unbounded retries cannot overflow.

Docs and reconcile semantics clarify that disk revocation via watch_skills requires "*"; explicit lists keep absent skills as error actions without pruning. _resolve_requests no longer treats absent as an incomplete run for prune suppression.

^{Reviewed by Cursor Bugbot for commit cc31b69. Bugbot is set up for automated code reviews on this repo. Configure here.}

[jeffdupont]

Follow-up to my GA review of #87. I reviewed f6bb491 against its base, xie/agent-skills at c547be2, not main. make test: 2166 passed, 11 skipped, exit 0. make typecheck is clean. I also reverted each fix on its own and re-ran the new tests. Without the none change, all three none tests fail. Without the exponent clamp, the backoff test fails with OverflowError. With the base branch's skills_fdv2.py put back, the no-option test and both retry-indefinitely tests fail. So the new tests do catch the bugs they're meant to.

Where the four #87 blockers stand:

• (1) Changes after none: resolved. This now matches the LaunchDarkly Python SDK's expect_changes().
• (3) Giving up after 10 failures: resolved. Removing max_consecutive_failures is not a breaking change. It never reached main (git grep on origin/main finds neither it nor FDv2SkillStore), so fix(skills) without a ! is right.
• (2) Skill-key grammar: not touched. Still open. Per your reply on #87, it needs an SDK-or-API decision, and it stays on the GA list until that's made.
• (4) Revocation for explicit lists: docs only. I think that's acceptable for 1.0. What 1.0 freezes is the conservative behaviour: absent keys keep their files and report an error. The likely fixes are additive and can ship in a minor: let watch_skills take a callable that returns the current refs, or add an opt-in prune. The one change that would be hard to make after 1.0 is making explicit lists prune absent keys by default, because 1.x would then delete files that 1.0 kept. If we never want that default, docs-only is fine. A few unscoped claims are left; details inline.

One thing to settle before merge:

1. The backoff options aren't validated, and without the budget, a zero or negative value now spins forever. With a fake requester that always fails: initial_backoff=0 made 773,754 attempts in one second; max_backoff=0 and max_backoff=-5 made about 775k each. failed stayed None. On the base branch, the same probe stops after 11 attempts with failed set. Against the real service, that's a fleet hammering LaunchDarkly over a config typo. JS #108 does the same (about 860 attempts/s). I'd validate both as positive and finite, as poll_interval already is, and add that to the spec. Inline.

Smaller notes:

• Fleet behaviour in an outage is fine for a hard outage. The cap is 30s with subtractive 50% jitter, the same as the LaunchDarkly Python SDK's FDv2 streaming (MAX_RETRY_DELAY = 30, JITTER_RATIO = 0.5, ldclient/impl/datasourcev2/streaming.py:55-57). I found two gaps, both older than this PR, but they now last for the whole outage instead of 10 attempts. First, Retry-After is used without jitter (skills_fdv2.py:1651-1655), so a fleet told Retry-After: 30 reconnects at the same moment. Second, backoff resets on every none, while the base SDK resets only after a connection has stayed open 60s (BACKOFF_RESET_INTERVAL = 60, streaming.py:56,118). A server that answers none and then drops each connection gets reconnects at about 1/s indefinitely. My probe with default backoff saw 27 connections in 20s. Both are spec questions.
• The clamp is correct at the boundaries. Attempts up to 1 give the base delay, and attempt 63 and above hit the clamp. float(2**62) is exact, and attempt 10,000 gives exactly 30.0.
• An oversized response is now retried forever. A body over MAX_RESPONSE_BYTES (64 MiB) is recoverable, so the store downloads up to 64 MiB again every 15-30s and never sets failed. The rewritten test at test_skills_fdv2.py:2777 now asserts exactly that. Before, the budget turned this into failed. It's unlikely, but it fails the same way every time, so it could be fatal. I haven't checked how large a real payload can get.
• Outage visibility. failed no longer signals an outage; only connection_failures and last_error do. A last-success timestamp on StoreDiagnostics would help health checks. That's additive, so it can come after 1.0.
• A stale comment at skills_fdv2.py:1711-1712 still mentions "exhausting its budget". #108 updated the same comment.
• Test parity with #108. JS has tests that Python doesn't, and spec #36 asks for two of them: waitForSkills running to its timeout during an outage, and fail/succeed/fail retrying at the first backoff step. JS also replaced the two 400-budget tests with "a 400 after a recoverable failure still repairs" and "the second 400 still stops"; this PR deletes them with no replacement.
• connection_failures means different things in the two languages. Python counts a goodbye after a complete answer as a failure: it sets last_error, and test_skills_fdv2.py:2406-2408 allows the count to read 1. JS exempts it (reachedServer, js skills-fdv2.ts:1810). This predates the PR, but the counter is now the main outage signal, so the two should agree and the spec should say which way.
• Spec: launchdarkly/ai-sdks-monorepo#36 matches both implementations on the two behaviour changes, the absence-by-name assertion and the finite-backoff assertion. I'd add four points there: the backoff options must be positive and finite; whether a goodbye after an answer counts toward connection_failures; when backoff resets; and whether Retry-After gets jitter.

JS counterpart: launchdarkly/js-ai-sdk#108 (same findings, noted there).

[XieX]

Thanks for the review. The items in the review body:

• Oversized response: now fatal, handled like a 422. failed is set, connection_failures doesn't move, and last known good is still served. Python a06237c, JS launchdarkly/js-ai-sdk@232396d.
• connection_failures across languages: Python now matches JS. A goodbye after a completed exchange isn't counted, doesn't set last_error, and logs at debug in both SDKs. A goodbye before any answer still counts and warns.
• Stale "budget" comment: fixed.
• Spec (launchdarkly/ai-sdks-monorepo#36): all four points are added: positive, finite backoff options; the goodbye rule; when backoff resets; and Retry-After. Retry-After is deliberately left unjittered for now. The base SDKs don't read it at all, and there's no room under the cap for additive jitter.
• Deferred: the last-success timestamp on StoreDiagnostics (additive, after 1.0).
• Skill-key grammar (feat: Agent Skills #87 blocker 2): this has been fixed on the API side.