Review follow-up to #3657 (squash-merged before these review fixes could
land on it). No behaviour change to the retry scoping itself — this
tightens correctness, docs, and test coverage around the
`without_retries` mechanism.
- **Correct the `without_retries` caching rationale.** The previous
docstring justified caching with "each clone creates a new `ContextVar`,
and dynamically-created `ContextVar`s are never garbage-collected." That
is false for this code path: `request_ctx` is only ever `.get()`, never
`.set()`, so it is reclaimed by refcounting on drop (measured: 500k
get-only clones leak ~0 KiB; the CPython leak only occurs with
`.set()`). Caching is kept because it avoids constructing a fresh
`HttpClient` on every `execute`/`proxy` (the hottest path) — the real,
stated justification now.
- **Preserve `_strict_response_validation` on the clone.** The
Stainless-generated `copy()` drops it, so the no-retry sibling silently
reverted to the default (`False`) even when the parent had it enabled.
The `copy()` override now threads it through `_extra_kwargs` alongside
`provider`, so the sibling differs from the parent only in
`max_retries`.
- **Document the scope boundary.** The `without_retries` docstring now
states that only `tools.execute` / `tools.proxy` are de-retried; other
non-idempotent writes (`auth_configs.create`/`update`/`delete`,
`mcp.update`/`delete`, `connected_accounts.delete`/`refresh`,
`link.create`) keep the default retries (most are naturally idempotent
on retry; the durable fix is backend idempotency keys).
- **Add regression coverage:** the clone preserves
`_strict_response_validation`, plus contract guards pinning the
Stainless `copy`/`_extra_kwargs`/`with_options` internals the override
relies on — so a `composio_client` regen that breaks the contract fails
as an obvious assertion rather than a cryptic `TypeError`.
- Minor: fix the `x-sdk-version` `"unknwon"` → `"unknown"` typo and
tighten `te.Self` typing on the cache field/property.
No changeset (Python-only change; changesets are TypeScript-only).
This PR:
- stops the Python SDK from silently retrying `tools.execute` and
`tools.proxy`, which are non-idempotent POST writes. The
Stainless-generated client retries POSTs by default (`max_retries=2`) on
read timeouts, 429, 5xx, and connection errors. A read timeout is unsafe
to retry — the request may already be in flight on the backend — so a
retry can duplicate the side effect, e.g. send an email twice. Reported
in https://github.com/ComposioHQ/composio/issues/3586
- routes both write paths through a new `client.without_retries` — a
cached, retry-disabled (`max_retries=0`) clone of the `HttpClient`.
Reads and lists keep the default retries, so resilience is unchanged for
idempotent calls
- overrides `HttpClient.copy()` to re-inject the required `provider`
keyword and re-aliases `with_options`, since the generated `copy()`
rebuilds via `self.__class__(...)` without `provider` — previously
`with_options(max_retries=0)` raised `TypeError` on the subclass
- caches the no-retry sibling instead of cloning per call: each clone
creates a new `ContextVar`, and dynamically-created `ContextVar`s are
never garbage-collected
- adds `tests/test_no_retry_writes.py`: writes hit the transport exactly
once on a retryable 5xx, reads still retry-then-succeed (proving the
scoping), and a regression guard for the `copy()` override
- interim fix only — the durable solution is idempotency keys, tracked
in https://github.com/ComposioHQ/composio/issues/3654 (gated on backend
support), which supersedes this once available
No changeset (Python-only change; changesets are TypeScript-only).
Alberto Schiabel