Ruby: max_retries: 0 made zero requests on an ungoverned GET (#532)

SPEC.md

@@ -108,11 +108,11 @@ END
 
 **Naming note:** `max_retries` means total attempts (including the initial request), not the number of retries after the first attempt. With `max_retries = 3`, the transport makes at most 3 attempts total (1 initial + 2 retries). This name is inherited from the shipping Ruby SDK; the behavior-model.json uses `retry.max` with identical semantics.
 
-**Per-operation retry ceiling.** Each operation carries a per-op `retry.max` in behavior-model.json (203 ops at `3`, 44 at `2`). **TypeScript and Swift** drive their retry loops directly from this per-op value, which is unambiguous there because neither exposes a numeric client-wide cap — only an on/off (`enableRetry`). Generated Go, Python, Kotlin (`BasecampConfig.maxRetries`), and Ruby's governed GET path (`config.max_retries`) expose a numeric client cap *and* honor the per-op value as a **ceiling**: `effective_attempts = min(client_cap, op_max)`. The ceiling can only reduce attempts below the client cap, never raise them, so a client that lowered its cap (e.g. to `1` to disable retries) is still honored; governed paths coerce the cap to at least one attempt (`min(max(1, cap), op_max)`), so `0` yields a single attempt rather than none. Because every op's `max` is ≤ the default cap of `3`, a default or raised client makes exactly the per-op number of attempts in every capped SDK — matching TS/Swift. Observable changes from the former client-wide behavior, by client configuration:
+**Per-operation retry ceiling.** Each operation carries a per-op `retry.max` in behavior-model.json (203 ops at `3`, 44 at `2`). **TypeScript and Swift** drive their retry loops directly from this per-op value, which is unambiguous there because neither exposes a numeric client-wide cap — only an on/off (`enableRetry`). Generated Go, Python, Kotlin (`BasecampConfig.maxRetries`), and Ruby's governed GET path (`config.max_retries`) expose a numeric client cap *and* honor the per-op value as a **ceiling**: `effective_attempts = min(client_cap, op_max)`. The ceiling can only reduce attempts below the client cap, never raise them, so a client that lowered its cap (e.g. to `1` to disable retries) is still honored. In Go, Python, and Ruby's governed path the cap is floored at one attempt before the ceiling applies (`min(max(1, cap), op_max)`), so a cap of `0` yields a single attempt rather than none whether or not the operation declares a retry block. Kotlin computes a different expression — `min(max(1, cap), op_max ?: cap)` — which is `0` for an ungoverned operation at a cap of `0`, but it lands on the same single attempt anyway because its loop consults the budget only after the first request has already gone out. Because every op's `max` is ≤ the default cap of `3`, a default or raised client makes exactly the per-op number of attempts in every capped SDK — matching TS/Swift. Observable changes from the former client-wide behavior, by client configuration:
 
 - **Default client (`max_retries = 3`):** only the **11 idempotent `max:2` operations** (account/gauge/preference writes plus two subscription-style POSTs: `UpdateAccountName`, `UpdateAccountLogo`, `RemoveAccountLogo`, `UpdateMyPreferences`, `DisableOutOfOffice`, `MarkAsRead`, `ToggleGauge`, `UpdateGaugeNeedle`, `DestroyGaugeNeedle`, `Subscribe`, `EnableCardColumnOnHold`) change — they now retry at most twice instead of three times. The other 195 retry-eligible ops are unaffected (`min(3, 3) = 3`).
 - **Client that raised its cap above 3:** **all 206 retry-eligible operations** are now clamped to their per-op `max` (195 to `3`, 11 to `2`) instead of retrying up to the raised cap. This is the intended meaning of a per-op ceiling and brings Go/Python into line with TS/Swift/Kotlin, which never retry beyond the per-op `max`. Go, Python, Kotlin, and Ruby's governed path all equally honor a caller who wants *fewer* attempts than the operation declares.
-- **Client that lowered its cap to `1`:** unchanged — the cap still wins (`min(cap, op_max) = cap`). A cap of `0` is coerced to one attempt on governed paths (see the `max_retries = 0` divergence note in §2). Go, Python, and Ruby's governed GET path consume `max` **and** `retry_on` (the declared status gate); only the emitted `base_delay_ms`/`backoff` remain inert per-op metadata for them (retained for parity — see `scripts/check-retry-metadata-parity.py`). Ruby remains GET-only: mutations never retry there, so per-op metadata governs only its reads.
+- **Client that lowered its cap to `1`:** unchanged — the cap still wins (`min(cap, op_max) = cap`). A cap of `0` is coerced to one attempt on every path, governed or not (see the `max_retries = 0` divergence note in §2). Go, Python, and Ruby's governed GET path consume `max` **and** `retry_on` (the declared status gate); only the emitted `base_delay_ms`/`backoff` remain inert per-op metadata for them (retained for parity — see `scripts/check-retry-metadata-parity.py`). Ruby remains GET-only: mutations never retry there, so per-op metadata governs only its reads.
 
 **Recommended default:** A connect timeout of 10 seconds is recommended but not a required config field. Only Ruby exposes this (Faraday `open_timeout = 10`); other SDKs use their HTTP library's default.
 
@@ -133,10 +133,10 @@ All validation errors are `BasecampError(code: "usage")` (see §6 error taxonomy
 1. Parse `base_url`. → `⊥ BasecampError(code: "usage")` if malformed.
 2. If `base_url` is not the default (`https://3.basecampapi.com`) and not localhost (§9), enforce HTTPS. → `⊥ BasecampError(code: "usage", message: "base URL must use HTTPS")` if scheme ≠ `https`.
 3. Validate `timeout > 0`. → `⊥ BasecampError(code: "usage")` otherwise.
-4. Validate `max_retries ≥ 1`. → `⊥ BasecampError(code: "usage")` otherwise. (`max_retries` is total attempts including the initial request; 0 would mean no request is made.) **Divergence:** the SDKs handle `max_retries = 0` in three distinct outcomes across four implementations, none of which is the spec's `⊥`:
+4. Validate `max_retries ≥ 1`. → `⊥ BasecampError(code: "usage")` otherwise. (`max_retries` is total attempts including the initial request; 0 would mean no request is made.) **Divergence:** the SDKs handle `max_retries = 0` in two distinct outcomes across four implementations, neither of which is the spec's `⊥`:
    - **Generated Go** (low-level `pkg/generated` client) and **Python** (sync + async): accept `0` as a compatibility exception and make a single attempt with no retry. Both reject a *negative* value as a configuration error (generated Go: `WithRetryConfig`/`doWithRetry` return a plain `error`; Python: `Config` raises `ValueError` at construction).
    - **Kotlin:** the builder rejects a *negative* value (`require(maxRetries >= 0)`) and accepts `0`, which the transport coerces to a single attempt (`config.maxRetries.coerceAtLeast(1)`).
-   - **Ruby:** `0` passes config validation. A **governed** GET (canonical operation ID present) coerces the cap to one attempt (`[config.max_retries, 1].max`) and makes a single request. An **ungoverned** GET keeps the old outcome: the retry loop's `break if attempt > max_retries` fires before the first request, so it makes **zero** requests and raises `Basecamp::ApiError("Request failed after 0 attempts")`.
+   - **Ruby:** `0` passes config validation and the transport coerces it to a single attempt (`[config.max_retries, 1].max`), matching Kotlin. The floor applies on every path: whether a request reaches the wire does not depend on whether the operation carries a declared retry block. A declared operation ceiling still clamps the floored cap downward.
    - **Hand-written Go** (`pkg/basecamp` client): rejects `0` — `NewClient` panics `"basecamp: max retries must be at least 1"` (its GET/download loops treat `MaxRetries` as the total attempt count with a minimum of 1).
 5. Validate `max_pages > 0`. → `⊥ BasecampError(code: "usage")` otherwise.
 6. Normalize `base_url`: strip trailing `/`.
@@ -660,7 +660,7 @@ Kotlin, and Ruby.
 
 - **TypeScript** implements the three-gate algorithm with the retry loop beneath the openapi-fetch middleware chain (the client's custom `fetch`), so attempts run to each operation's declared `retry.max` and network errors retry under the same idempotency gate; caller aborts and request timeouts are terminal. **Kotlin** implements the three-gate algorithm for both HTTP status and network-error retries (POST retries only when `idempotent: true`, full exponential backoff): one eligibility gate covers both failure shapes, with the whole-request timeout (Ktor's `HttpRequestTimeoutException`) deliberately not retried and auth headers re-attached per attempt.
 - **Go** implements the three-gate on its generated operation path: it retries operations classified idempotent at generation time — GET/HEAD by method, plus any operation carrying `x-basecamp-idempotent` (the naturally-idempotent PUT/DELETE mutations like `UpdateProject`/`TrashProject`, and the flagged-idempotent POSTs like `CompleteTodo`) — with exponential backoff; non-idempotent operations (e.g. `CreateTodo`) are single-attempt. The separate hand-written `doRequestURL` helper remains GET-only for ordinary retries, with a mutation-specific single re-attempt after successful 401 token refresh.
-- **Ruby** is stricter: only GET retries; all non-GET methods do not retry. Governed GETs (those carrying their canonical operation ID) are bounded by the per-op ceiling and status-gated on the declared `retryOn`; ungoverned GETs (`get_absolute`, OAuth discovery) keep the taxonomy-driven pre-metadata contract. Ruby is acceptably conservative.
+- **Ruby** is stricter: only GET retries; all non-GET methods do not retry. Governed GETs (those carrying their canonical operation ID) are bounded by the per-op ceiling and status-gated on the declared `retryOn`; ungoverned GETs (`get_absolute`, OAuth discovery) keep the taxonomy-driven pre-metadata status contract, under the same floored caller cap. Ruby is acceptably conservative.
 - **Swift** implements the three-gate algorithm: the transport retries only when the method is naturally idempotent (GET/HEAD/PUT/DELETE) **or** the operation is marked `idempotent: true`, so non-idempotent POSTs like `CreateProject` are attempted exactly once while the seven idempotent POSTs (`CompleteTodo`, `CreateBookmark`, `EnableCardColumnOnHold`, `PauseQuestion`, `PrioritizeAssignment`, `Subscribe`, `SubscribeToCardColumn`) keep retrying. The gate covers both retry paths — HTTP status (`429`/`503`) and network errors — so Swift retries network errors but only for retry-eligible operations. A network error is classified by *meaning*, not by type: a `Transport` that reports connectivity failure as the SDK's own `BasecampError.network` reaches the retry branch exactly as a raw `URLError` does (#567). `Transport` is `public`, so that normalization is the natural implementation and must not be the one that disables retry. Any other `BasecampError` out of the transport (`.auth`, `.usage`, `.api`, …) stays terminal on sight, and the non-HTTP-response guard raises a distinct internal error so a deterministic programming fault is never mistaken for a transport blip. `BaseService` threads the per-operation flag from generated `Metadata` into the transport; the naturally-idempotent method set is allowlisted so PATCH/OPTIONS and future methods stay fail-closed.
 - The spec prescribes the three-gate algorithm.
 
@@ -1316,7 +1316,7 @@ Attempt budget per SDK — disabling retry (each SDK's spelling of `enable_retry
 |-----|--------|
 | Go | `MaxRetries` as total attempts (hand-written client rejects < 1) |
 | Python | `max_retries` as total attempts, floored at one (`max_retries: 0` still sends one attempt) |
-| Ruby | `max_retries` as total attempts, floored at one for downloads (`max_retries: 0` still sends one attempt; the general ungoverned GET path's zero-attempt behavior is tracked separately) |
+| Ruby | `max_retries` as total attempts, floored at one on every path — downloads, governed GETs and ungoverned GETs alike (`max_retries: 0` still sends one attempt) |
 | Kotlin | `maxRetries` as total attempts, floored at one, gated on `enableRetry`; an accepted `maxRetries = 0` still sends exactly one attempt |
 | TypeScript | Fixed three-attempt policy when `enableRetry` is true; one attempt when false. No public numeric knob. |
 | Swift | Fixed three-attempt policy when `enableRetry` is true; one attempt when false. No public numeric knob. |
@@ -3352,7 +3352,7 @@ Every operation has a `retry` block, including non-idempotent POSTs. For non-ide
 | TypeScript | Three-gate: POST retries only when `idempotent: true`. Retries on `retry_on` set from metadata to each operation's declared `max`, with the retry loop beneath the openapi-fetch middleware chain as the client's custom `fetch`. Network errors retry under the same idempotency gate; caller aborts and request timeouts are terminal. |
 | Kotlin | Three-gate for both HTTP status and network-error retries: POST retries only when `idempotent: true`, full exponential backoff. Network errors retry through the same eligibility gate; the whole-request timeout (Ktor's `HttpRequestTimeoutException`) is deliberately not retried. |
 | Go | Generated operation path retries operations classified idempotent at generation time — GET/HEAD by method, plus any operation carrying `x-basecamp-idempotent` (naturally-idempotent PUT/DELETE mutations like `UpdateProject`/`TrashProject`, and flagged-idempotent POSTs like `CompleteTodo`) — with exponential backoff; non-idempotent operations (e.g. `CreateTodo`) are single-attempt. The separate hand-written `doRequestURL` helper remains GET-only for ordinary retries, with a mutation-specific single re-attempt after successful 401 token refresh. |
-| Ruby | Simplified: only GET retries. All non-GET methods never retry. Governed GETs gate status retries on the declared `retryOn` and bound attempts by `min(config.max_retries, operation max)`; ungoverned traffic (no operation ID: `get_absolute`, OAuth) keeps the taxonomy-driven contract. |
+| Ruby | Simplified: only GET retries. All non-GET methods never retry. Governed GETs gate status retries on the declared `retryOn` and bound attempts by `min(config.max_retries, operation max)`; ungoverned traffic (no operation ID: `get_absolute`, OAuth) keeps the taxonomy-driven status contract, under the same floored caller cap. |
 | Python | Three-gate, sync and async: `_mutation()` retries only when `behavior-model` metadata classifies the operation retryable, so non-idempotent POSTs are single-attempt; GETs always retry. Gate 3 uses the operation's declared `retry_on` and `max`. Non-Smithy traffic (`get_absolute()`, Launchpad authorization) passes no operation id and keeps the pre-Smithy contract. |
 | Swift | Three-gate: retries when the method is naturally idempotent (GET/HEAD/PUT/DELETE) or the operation is marked `idempotent: true`; non-idempotent POSTs make a single attempt. Gate covers both HTTP status and network-error retries, so Swift *does* retry network errors, gated by idempotency. |
 

ruby/README.md

@@ -441,14 +441,14 @@ result = account.download_url(url)
 
 ## Retry Behavior
 
-Only plain GET requests retry — automatically, with exponential backoff. Mutation operations (POST, PUT, DELETE) do **not** retry to prevent data duplication, and the raw upload and download paths skip the retry loop entirely (the upload path is strictly one request; the download hop keeps only the one-shot 401 replay below).
+Only plain GET requests retry — automatically, with exponential backoff. Mutation operations (POST, PUT, DELETE) do **not** retry to prevent data duplication, and the raw upload path skips the retry loop entirely — it is strictly one request. The download flow's authenticated first hop does retry, but under its own declared status set (`429`, `502`, `503`, `504` — never `500`); its second hop, the fetch from the signed URL, is a single unauthenticated request.
 
-- **Which errors**: Retry keys off the error's `retryable?` classification, not a declared status list — 429 (rate limit), 500, 502, 503, 504, and any other 5xx all retry, as does `NetworkError` (connection failures, including DNS and connect-phase timeouts). Read timeouts are the exception: Faraday surfaces them as a status-less `ApiError` with `retryable? == false`, so a GET that times out mid-response fails on the first attempt. 400, 401, 403, 404, and 422 never retry.
-- **`max_retries`**: Total request attempts for GET requests, including the initial request — the default `3` means one initial attempt plus two retries. **`max_retries: 0` sends zero requests** and raises `Basecamp::ApiError` (`"Request failed after 0 attempts"`).
+- **Which errors**: A GET issued through a generated service carries its operation ID and is **governed** — status retries are gated on the statuses that operation declares, which is `[429, 503]` for every operation in the current metadata. A governed GET does **not** retry 500. Only the handful of GETs that carry no operation ID (`get_absolute`, OAuth discovery) fall back to the error taxonomy, where 429, 500, 502, 503, 504, and any other 5xx all retry. `NetworkError` (connection failures, including DNS and connect-phase timeouts) retries on both paths, since it has no status to gate on. Read timeouts are the exception: Faraday surfaces them as a status-less `ApiError` with `retryable? == false`, so a GET that times out mid-response fails on the first attempt. 400, 401, 403, 404, and 422 never retry.
+- **`max_retries`**: Total request attempts for GET requests, including the initial request — the default `3` means one initial attempt plus two retries. `max_retries: 0` is floored to a single attempt rather than sending zero requests.
 - **Backoff**: Exponential with jitter — `base_delay * 2^(attempt - 1) + rand * max_jitter` — uncapped, bounded in practice by the attempt budget.
 - **Rate limits**: A 429's `Retry-After` header overrides the calculated backoff. Only 429 carries it: 5xx and network errors always use the exponential backoff.
 - **401 responses**: With a refresh-capable token provider, the SDK refreshes the token and replays the request **once** — for all methods, including mutations — outside the `max_retries` budget. A second 401 is surfaced. The raw upload path has no 401 replay.
-- **Per-operation metadata**: The retry policy operations declare (`retry_on` statuses, per-operation `max`) is inert in Ruby — every API GET issued through the client, including the Launchpad authorization fetch, rides the same classification-based loop bounded by `config.max_retries` alone. (The download flow's redirect hop and OAuth discovery use their own single-attempt transports.)
+- **Per-operation metadata**: Every GET a generated service issues passes its canonical operation ID, so essentially all SDK reads are **governed**: attempts are bounded by `min(config.max_retries, operation max)` and status retries are gated on the operation's declared `retry_on`. The GETs that carry no operation ID — `get_absolute` and the Launchpad authorization fetch it backs — are **ungoverned** and ride the classification-based loop bounded by `config.max_retries` alone. The declared `base_delay_ms` and `backoff` are inert in Ruby either way: the backoff is always the client's. (OAuth discovery uses its own single-attempt transport.)
 - **`retryable?`**: Unlike SDKs where the error classification is only a hint for your own code, in Ruby an error's `retryable?` (and `retry_after`) is exactly what the transport acts on for GET requests.
 
 ## Error Handling