From 62fe384bc6eb40246eb7aa7118f4f04f63be3d78 Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Mon, 5 Oct 2026 15:40:26 -0400 Subject: [PATCH 1/2] fix(client): reset connection_failures when a store that gave up is started again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_rearm_waiters` reset the internal `_failures` count but not `diagnostics.connection_failures`, so a restarted store reported the old run's failures until its new run failed or succeeded: after two recoverable failures and a 401, `start()` left `failed` None with `connection_failures == 2` and nothing failed in the new run. That counter is the outage signal now that recoverable failures retry indefinitely, so a healthy restart read as a store already riding out an outage. The TypeScript SDK resets it in `start()`. `test_a_restart_resets_the_failure_count` could not see this: it reads the counter only after the new run's first failure, which overwrites the stale value. The new test holds the new run's first request open and reads the counter right after `start()`; it fails with `2 == 0` when the reset is reverted. Spec: launchdarkly/ai-sdks-monorepo#40 (TESTING.md §3.25). Co-Authored-By: Claude Opus 5.5 --- packages/client/README.md | 4 +- packages/client/agents.md | 3 +- .../src/launchdarkly_ai_server/skills_fdv2.py | 6 ++- packages/client/tests/test_skills_fdv2.py | 47 +++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/packages/client/README.md b/packages/client/README.md index 1337c8fe..bd770abf 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -628,8 +628,8 @@ HTTP 422 when it will not serve one. - **Not the empty case:** an environment with zero skills is served an empty payload that commits normally. - **Recovery:** once the cause is fixed, call `start()` on the same store. Only `close()` is - final: a restart clears `failed`, resets the backoff, and held content stays readable - throughout. Restarting the process also works. + final: a restart clears `failed`, resets the backoff and `connection_failures`, and held + content stays readable throughout. Restarting the process also works. **Nothing above the store changes.** The accessors, verification, and `write_skills` see raw objects through the `SkillStore` interface and cannot tell which store produced them. diff --git a/packages/client/agents.md b/packages/client/agents.md index 8e1eb533..a57c3389 100644 --- a/packages/client/agents.md +++ b/packages/client/agents.md @@ -264,7 +264,8 @@ payload assigned to it, and chose a non-400 4xx because LD SDKs treat those as t `_classify_status` returns a `_FatalTransportError` and the normal give-up path runs: `failed` and `last_error` are set, `wait_for_skills` returns `False` at once, and `connection_failures` is left untouched (it counts consecutive *recoverable* failures; an -existing count is kept, not zeroed). +existing count is kept, not zeroed, until `start()` runs delivery again and `_rearm_waiters` +resets it). **The 422 message matches the TypeScript SDK's word for word, and stays short.** It names the one cause a customer can fix — a view-scoped SDK key — and refers every other case to diff --git a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py index 25f42918..456b2919 100644 --- a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py +++ b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py @@ -1503,12 +1503,14 @@ def start(self) -> FDv2SkillStore: def _rearm_waiters(self) -> None: """ Resets per-run state for a store being started again after it gave up: - the ended-delivery flag, ``failed``, and the failure count. A payload - already held still answers ``wait_for_skills``. Call with the lock held. + the ended-delivery flag, ``failed``, and the failure count, including + the one ``diagnostics`` reports. A payload already held still answers + ``wait_for_skills``. Call with the lock held. """ self._delivery_ended.clear() self._failed_reason = None self._failures = 0 + self._reader.diagnostics.connection_failures = 0 self._backoff_attempts = 0 if not self._first_payload.is_set(): self._released.clear() diff --git a/packages/client/tests/test_skills_fdv2.py b/packages/client/tests/test_skills_fdv2.py index 1864229b..0fbe85b8 100644 --- a/packages/client/tests/test_skills_fdv2.py +++ b/packages/client/tests/test_skills_fdv2.py @@ -2168,6 +2168,53 @@ def test_a_restart_resets_the_failure_count(self) -> None: # One failure in the new run, not three. assert store.diagnostics.connection_failures == 1 + def test_a_restart_reports_no_failures_before_the_new_run_has_any(self) -> None: + """The reported count starts over in ``start()``, not at the next failure. + + Read only after the new run fails, a stale count is overwritten and looks + reset. Read while the new run's first request is still in flight, it + would show the old run's failures on a store whose ``failed`` is None. + """ + + class _HoldsWhenScriptEnds(_ScriptedRequester): + def __init__(self, *outcomes: Any) -> None: + super().__init__(*outcomes) + self.release = threading.Event() + + def poll(self, basis: str | None, etag: str | None) -> Any: + if not self.outcomes: + self.calls.append((basis, etag)) + self.release.wait(timeout=10) + raise _RecoverableTransportError("released") + return super().poll(basis, etag) + + requester = _HoldsWhenScriptEnds( + _RecoverableTransportError("x"), + _RecoverableTransportError("x"), + _FatalTransportError("401"), + ) + store = FDv2SkillStore( + SDK_KEY, + mode="poll", + initial_backoff=0.001, + max_backoff=0.002, + _requester=requester, + ) + try: + store.start() + assert wait_until(lambda: store.failed is not None) + assert store.diagnostics.connection_failures == 2 + + calls = len(requester.calls) + store.start() + assert store.diagnostics.connection_failures == 0 + assert wait_until(lambda: len(requester.calls) > calls) + assert store.failed is None + assert store.diagnostics.connection_failures == 0 + finally: + requester.release.set() + store.close() + def test_a_404_stops_delivery_immediately(self, endpoint: Any) -> None: """A 404 means the endpoint does not exist for this credential. From a068cbc2ee57e925e1806d760ce71ab2a9ae51d1 Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Tue, 6 Oct 2026 10:32:10 -0400 Subject: [PATCH 2/2] docs(client): say a restarted store's wait_for_skills waits again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docstring listed a give-up as ending delivery with no hint that start() undoes it. Matches TESTING.md §3.25 (ai-sdks-monorepo#40). Co-Authored-By: Claude Opus 5.5 --- packages/client/src/launchdarkly_ai_server/skills_fdv2.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py index 456b2919..fa19aef6 100644 --- a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py +++ b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py @@ -1559,7 +1559,8 @@ def wait_for_skills(self, timeout: float = 10.0) -> bool: has skills; see ``diagnostics``. Returns ``False`` on timeout, or early if delivery ends first (``close``, - or a failure that will not be retried). + or a failure that will not be retried). A store that gave up waits again + once ``start()`` runs delivery again; only ``close`` is final. """ self._released.wait(timeout=timeout) return self._first_payload.is_set()