diff --git a/SPEC.md b/SPEC.md index 48e218585..af8ea1625 100644 --- a/SPEC.md +++ b/SPEC.md @@ -112,7 +112,7 @@ END - **Default client (`max_retries = 3`):** only the **9 idempotent `max:2` operations** (account/gauge/preference writes: `UpdateAccountName`, `UpdateAccountLogo`, `RemoveAccountLogo`, `UpdateMyPreferences`, `DisableOutOfOffice`, `MarkAsRead`, `ToggleGauge`, `UpdateGaugeNeedle`, `DestroyGaugeNeedle`) change — they now retry at most twice instead of three times. The other 178 retry-eligible ops are unaffected (`min(3, 3) = 3`). - **Client that raised its cap above 3:** **all 187 retry-eligible operations** are now clamped to their per-op `max` (178 to `3`, 9 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 and Python are, however, the only two that also honor a caller who wants *fewer* attempts than the operation declares — see the Kotlin note above.) -- **Client that lowered its cap to `0`/`1`:** unchanged — the cap still wins (`min(cap, op_max) = cap`). Go/Python consume only `max`; the emitted `base_delay_ms`/`backoff`/`retry_on` remain inert per-op metadata for them (retained for parity — see `scripts/check-retry-metadata-parity.py`). Ruby retries GET only and bounds its loop by `config.max_retries` alone, so the per-op `max` is not enforced there at all — the opposite gap from Kotlin's. All per-op retry fields are inert in Ruby. +- **Client that lowered its cap to `0`/`1`:** unchanged — the cap still wins (`min(cap, op_max) = cap`). Go and Python 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 retries GET only and bounds its loop by `config.max_retries` alone, so the per-op `max` is not enforced there at all — the opposite gap from Kotlin's. All per-op retry fields are inert in Ruby. **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. @@ -403,20 +403,39 @@ If `behavior-model.json` marks an operation with `idempotent: true`, the POST be The error must be retryable. Two categories qualify: -- **HTTP status retry:** Response status is in the transport's retryable set. The `behavior-model.json` specifies `retry_on: [429, 503]` for all operations. Implementations may expand this set to include other 5xx statuses (500, 502, 504). +- **HTTP status retry:** Response status is in the operation's **declared** retryable set. `behavior-model.json` specifies `retry_on: [429, 503]` for all operations. The declared set is **exhaustive**: a status outside it — including 500, 502, and 504 — is not retried and is surfaced to the caller on the first attempt. An implementation may still *classify* those statuses as retryable in its error taxonomy (§6); that is a caller-facing hint and must not widen the transport's gate. - **Network error retry:** Connection failures, timeouts, and DNS errors (no HTTP response received) are retryable. These correspond to `BasecampError(code: "network", retryable: true)` in §6. **Divergence:** **Go, Python, and Swift** retry network errors for retry-eligible operations — including idempotent mutations — with Swift and Go gating on operation idempotency, so a non-idempotent POST is attempted once. **Ruby** retries network errors too, but only on GET: its transport routes every non-GET to a single-attempt path, so no mutation ever sees a network retry. **TypeScript** retries only after receiving an HTTP response; **Kotlin** surfaces network errors immediately without retry. The spec prescribes network error retry as the target behavior. **Non-retryable statuses (never retry regardless of method):** 401, 403, 404, 400, 422. -**Gate 3 status-set fidelity `[CONFLICT]`.** Gates 1 and 2 read `behavior-model.json`. Gate 3's -parameters are honored unevenly. The per-operation attempt **ceiling** is consumed by every SDK -except **Ruby**, which bounds its GET-only loop by `config.max_retries` alone and never reads the -operation's `max` (see *Per-operation retry ceiling* in §2). The declared **status set** is honored -by fewer still. **Go and Ruby retry statuses the spec never declared retryable**: Go's -`isRetryableStatus` covers `{429, 500, 502, 503, 504}` regardless of the operation's declared -`retry_on`, and Ruby's loop keys off `retryable?`, which `errors.rb` sets for 500/502/503/504. -**Python** has no status gate at all — its loop keys off the `retryable` flag `errors.py` sets for -the same statuses. TypeScript, Kotlin, and Swift do gate on the declared set. +**Gate 3 consumption `[CONFLICT]`.** Gates 1 and 2 read `behavior-model.json`. Gate 3's parameters +are consumed unevenly: + +| | Gates status retry on the declared `retryOn` | Honors a caller asking for *fewer* attempts than the operation declares | +|---|---|---| +| Go | yes | yes — `min(caller_cap, operation_max)` | +| Python | yes | yes — `min(caller_cap, operation_max)` | +| TypeScript | yes | n/a — exposes no numeric cap (only `enableRetry`), so the operation value is the only input. Additionally caps at one retry (waiver 2B.1) | +| Swift | yes | n/a — exposes no numeric cap (only `enableRetry`) | +| **Kotlin** | yes | **no** — `BasecampConfig.maxRetries` *is* numeric, but `opRetry?.maxRetries ?: config.maxRetries` lets the operation value override it, so a caller who lowers the cap is ignored | +| **Ruby** | **no** — the loop keys off `retryable?`, which `errors.rb` sets for 500/502/503/504, so a 500 is retried on GET | n/a for the ceiling — bounded by `config.max_retries` alone, so the operation's `max` is never enforced at all | + +Only **Kotlin** turns the ceiling into a floor: it is the one SDK with both a numeric caller cap and +a resolution order that discards it. TypeScript and Swift have no cap to discard, and Ruby's gap is +the mirror image — it honors the caller and ignores the operation. Tracked in #485. + +**Ungoverned traffic.** A transport may carry requests that are not Smithy operations — today the +Launchpad authorization request. Those carry no behavior-model metadata, and the declared policy +must **not** be applied to them: they keep whatever contract the SDK gives non-generated traffic. +Python enforces this structurally — `get_absolute()` passes no operation id, so both the status gate +and the attempt ceiling no-op — and pins it with sync and async regressions that fail if generated +policy ever reaches OAuth traffic. + +**Enforcement.** `scripts/check-retry-metadata-parity.py` asserts every SDK's emitted per-operation +retry metadata equals `behavior-model.json`, and records which fields each SDK actually consumes at +runtime. It deliberately does not assert *effective behavior* parity, since TypeScript's one-retry +cap and Ruby's GET-only transport are intentional. Effective Gate 3 behavior is pinned by tests in +Go and Python. ### Cross-SDK Divergence `[CONFLICT]` @@ -1610,18 +1629,15 @@ 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. Chains at most 1 retry via `fetch(retryRequest)` which bypasses middleware (waiver 2B.1). | | Kotlin | Three-gate for HTTP status retries: POST retries only when `idempotent: true`, full exponential backoff. Does not retry network errors (transport exceptions returned immediately). | | 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. Ruby retries on any error with `retryable? == true`. | -| 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. Retries on any error with `retryable == True`. | +| Ruby | Simplified: only GET retries. All non-GET methods never retry. Ruby retries on any error with `retryable? == true`, so it does not gate on the declared `retryOn` — a 500 is retried on GET. | +| 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 (unlike Kotlin/TS), gated by idempotency. | The table above describes **Gate 1 and Gate 2** — *whether* an operation retries. Gate 3's parameters -are tracked separately, and neither is uniform. The per-operation attempt ceiling is honored by Go, -Python, TypeScript, Swift and Kotlin — but **Ruby never consults it**: `request_with_retry` loops -until `@config.max_retries` (`ruby/lib/basecamp/http.rb`) without reading operation metadata, so a -Ruby caller who raises the cap above 3 exceeds the operation's declared `max`. (Kotlin honors the -ceiling but lets it override a *lower* caller cap — see #485.) The declared `retry_on` set is not -uniform either: Go, Python, and Ruby all retry a superset of it. See the Gate 3 status-set note at -§7 above. +are tracked separately: five SDKs gate status retry on the declared `retryOn` (Ruby does not), and +only Go and Python honor a caller asking for *fewer* attempts than an operation declares — Kotlin +lets the operation value override its numeric cap, TypeScript and Swift expose no such cap, and Ruby +never consults the operation's `max`. See the Gate 3 consumption table in §7 above. ### Integer Precision (§10) diff --git a/go/pkg/basecamp/generated_retry_statusset_test.go b/go/pkg/basecamp/generated_retry_statusset_test.go new file mode 100644 index 000000000..97684952b --- /dev/null +++ b/go/pkg/basecamp/generated_retry_statusset_test.go @@ -0,0 +1,146 @@ +package basecamp + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/basecamp/basecamp-sdk/go/pkg/generated" +) + +// These tests pin Gate 3's STATUS SET, the half the per-operation ceiling work +// did not cover. behavior-model.json declares retry_on: [429, 503] for all 226 +// operations; the generated client previously used a global +// {429, 500, 502, 503, 504} allowlist, so it retried three statuses the spec +// never declared retryable. The declared set is now emitted as +// operationRetryOn and consulted by operationId. +// +// ParseHTTPError still classifies 500/502/503/504 as retryable errors for the +// caller's benefit. That is a caller-facing hint, and these tests exist partly +// to pin that it does NOT widen the transport's gate. +// +// They stay outside pkg/generated per the repo rule that generated code carries +// no hand-written tests. + +func statusRetryConfig(maxRetries int) generated.RetryConfig { + return generated.RetryConfig{ + MaxRetries: maxRetries, + BaseDelay: time.Millisecond, + MaxDelay: 2 * time.Millisecond, + Multiplier: 2.0, + } +} + +func countingStatusHandler(status int, counter *int32) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(counter, 1) + w.WriteHeader(status) + } +} + +func TestGeneratedRetry_HonorsDeclaredStatusSet(t *testing.T) { + cases := []struct { + name string + status int + wantAttempts int32 + }{ + {"429 is declared retryable", http.StatusTooManyRequests, 3}, + {"503 is declared retryable", http.StatusServiceUnavailable, 3}, + {"500 is not declared retryable", http.StatusInternalServerError, 1}, + {"502 is not declared retryable", http.StatusBadGateway, 1}, + {"504 is not declared retryable", http.StatusGatewayTimeout, 1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var attempts int32 + server := httptest.NewServer(countingStatusHandler(tc.status, &attempts)) + defer server.Close() + + client, err := generated.NewClient(server.URL, generated.WithRetryConfig(statusRetryConfig(3))) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + // CompleteTodo is an idempotent POST with maxAttempts 3, so the only + // thing varying across these cases is the status. + resp, err := client.CompleteTodo(context.Background(), "99999", 100) + if err != nil { + t.Fatalf("CompleteTodo returned error: %v", err) + } + if resp.StatusCode != tc.status { + t.Errorf("got status %d, want %d", resp.StatusCode, tc.status) + } + _ = resp.Body.Close() + + if got := atomic.LoadInt32(&attempts); got != tc.wantAttempts { + t.Errorf("made %d attempts for status %d, want %d", got, tc.status, tc.wantAttempts) + } + }) + } +} + +// The read path must be gated too: a GET is retry-eligible by method, so +// without the status gate a 500 would retry to exhaustion. +func TestGeneratedRetry_ReadPathHonorsDeclaredStatusSet(t *testing.T) { + var attempts int32 + server := httptest.NewServer(countingStatusHandler(http.StatusInternalServerError, &attempts)) + defer server.Close() + + client, err := generated.NewClient(server.URL, generated.WithRetryConfig(statusRetryConfig(3))) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + resp, err := client.GetAccount(context.Background(), "99999") + if err != nil { + t.Fatalf("GetAccount returned error: %v", err) + } + _ = resp.Body.Close() + + if got := atomic.LoadInt32(&attempts); got != 1 { + t.Errorf("made %d attempts on a 500, want 1 (500 is not in the declared retry_on set)", got) + } +} + +// The generated table must carry the declared set verbatim. This is the raw +// metadata half; the behavioral half is above. +func TestGeneratedRetryOn_MatchesDeclaredSet(t *testing.T) { + for _, operation := range []string{"CompleteTodo", "GetAccount", "UpdateGaugeNeedle"} { + retryOn, ok := generated.GetOperationRetryOn(operation) + if !ok { + t.Errorf("%s missing from operationRetryOn", operation) + continue + } + if len(retryOn) != 2 || retryOn[0] != 429 || retryOn[1] != 503 { + t.Errorf("%s retryOn = %v, want [429 503]", operation, retryOn) + } + } +} + +// An operation's declared set is authoritative in both directions. A +// present-but-EMPTY retryOn means "never retry on any status"; only an absent +// entry falls back to the default. isRetryableStatus is unexported, so this +// exercises the exported accessor plus the documented fallback rule. +func TestGeneratedRetryOn_EmptySetIsNotAbsent(t *testing.T) { + // Every generated operation carries a set today, so the fallback is only + // reachable for an unknown id. Pin that it IS the default, so the two cases + // stay distinguishable if an operation ever declares retryOn: []. + if _, ok := generated.GetOperationRetryOn("NoSuchOperation"); ok { + t.Fatal("an unknown operation must not be present in operationRetryOn") + } + + // And pin that no shipped operation declares an empty set, which is what + // makes the fallback unreachable from a generated call site today. + for _, operation := range []string{"CompleteTodo", "GetAccount", "UpdateGaugeNeedle"} { + retryOn, ok := generated.GetOperationRetryOn(operation) + if !ok { + t.Fatalf("%s missing from operationRetryOn", operation) + } + if len(retryOn) == 0 { + t.Errorf("%s declares an empty retryOn; it would never retry on any status", operation) + } + } +} diff --git a/go/pkg/generated/client.gen.go b/go/pkg/generated/client.gen.go index cef7b41cc..c3ff4d468 100644 --- a/go/pkg/generated/client.gen.go +++ b/go/pkg/generated/client.gen.go @@ -4015,18 +4015,32 @@ func WithLogger(logger *slog.Logger) ClientOption { } } -// isRetryableStatus returns true if the HTTP status code indicates a retryable error. -func isRetryableStatus(statusCode int) bool { - switch statusCode { - case http.StatusTooManyRequests, // 429 - http.StatusInternalServerError, // 500 - http.StatusBadGateway, // 502 - http.StatusServiceUnavailable, // 503 - http.StatusGatewayTimeout: // 504 - return true - default: - return false +// defaultRetryOn is behavior-model.json's declared default, applied only when an +// operation carries no x-basecamp-retry.retryOn — which cannot happen for a +// generated call site. +var defaultRetryOn = []int{http.StatusTooManyRequests, http.StatusServiceUnavailable} + +// isRetryableStatus reports whether the status is in the operation's DECLARED +// retry_on set. The set is per-operation and exhaustive: a status the spec never +// declared retryable — including 500, 502, and 504 — is not retried, and is +// surfaced to the caller on the first attempt. ParseHTTPError still classifies +// those statuses as retryable errors for the caller's benefit; that is a +// caller-facing hint and deliberately does not widen this gate. +func isRetryableStatus(statusCode int, operationId string) bool { + // Only an ABSENT entry falls back. A present-but-empty set is an operation + // declaring "never retry on any status", which is not the same as declaring + // nothing — collapsing the two would silently re-enable 429/503 retries on an + // operation that opted out. + retryOn, ok := operationRetryOn[operationId] + if !ok { + retryOn = defaultRetryOn + } + for _, status := range retryOn { + if status == statusCode { + return true + } } + return false } // captureReplayBody inspects the finalized first-attempt request and returns a @@ -4285,7 +4299,7 @@ func (c *Client) doWithRetry(ctx context.Context, buildRequest func() (*http.Req } // Success or non-retryable status - if !isRetryableStatus(resp.StatusCode) { + if !isRetryableStatus(resp.StatusCode, operationId) { return resp, nil } @@ -20290,6 +20304,246 @@ var operationRetryMax = map[string]int{ "UpdateWebhook": 3, } +// operationRetryOn maps operation IDs to their declared retryable status set +// from the x-basecamp-retry extension. Like operationRetryMax it is a separate +// map rather than a field on OperationMetadata, so wiring it up does not change +// that exported struct's shape. +var operationRetryOn = map[string][]int{ + "GetAccount": {429, 503}, + "RemoveAccountLogo": {429, 503}, + "UpdateAccountLogo": {429, 503}, + "UpdateAccountName": {429, 503}, + "CreateAttachment": {429, 503}, + "GetEverythingBoosts": {429, 503}, + "DeleteBoost": {429, 503}, + "GetBoost": {429, 503}, + "SetCardColumnColor": {429, 503}, + "DisableCardColumnOnHold": {429, 503}, + "EnableCardColumnOnHold": {429, 503}, + "DeleteWormhole": {429, 503}, + "UpdateWormhole": {429, 503}, + "CreateWormhole": {429, 503}, + "ListMessageTypes": {429, 503}, + "CreateMessageType": {429, 503}, + "DeleteMessageType": {429, 503}, + "GetMessageType": {429, 503}, + "UpdateMessageType": {429, 503}, + "CreateTool": {429, 503}, + "ListWebhooks": {429, 503}, + "CreateWebhook": {429, 503}, + "GetCard": {429, 503}, + "UpdateCard": {429, 503}, + "MoveCard": {429, 503}, + "RepositionCardStep": {429, 503}, + "CreateCardStep": {429, 503}, + "GetCardColumn": {429, 503}, + "UpdateCardColumn": {429, 503}, + "ListCards": {429, 503}, + "CreateCard": {429, 503}, + "UnsubscribeFromCardColumn": {429, 503}, + "SubscribeToCardColumn": {429, 503}, + "GetCardStep": {429, 503}, + "UpdateCardStep": {429, 503}, + "SetCardStepCompletion": {429, 503}, + "GetCardTable": {429, 503}, + "CreateCardColumn": {429, 503}, + "MoveCardColumn": {429, 503}, + "GetEverythingCompletedCards": {429, 503}, + "GetEverythingNoDueDateCards": {429, 503}, + "GetEverythingNotNowCards": {429, 503}, + "GetEverythingOpenCards": {429, 503}, + "GetEverythingOverdueCards": {429, 503}, + "GetEverythingUnassignedCards": {429, 503}, + "ListCampfires": {429, 503}, + "GetCampfire": {429, 503}, + "ListChatbots": {429, 503}, + "CreateChatbot": {429, 503}, + "DeleteChatbot": {429, 503}, + "GetChatbot": {429, 503}, + "UpdateChatbot": {429, 503}, + "ListCampfireLines": {429, 503}, + "CreateCampfireLine": {429, 503}, + "DeleteCampfireLine": {429, 503}, + "GetCampfireLine": {429, 503}, + "UpdateCampfireLine": {429, 503}, + "ListCampfireUploads": {429, 503}, + "CreateCampfireUpload": {429, 503}, + "GetEverythingCheckins": {429, 503}, + "ListPingablePeople": {429, 503}, + "ListClientApprovals": {429, 503}, + "GetClientApproval": {429, 503}, + "ListClientCorrespondences": {429, 503}, + "GetClientCorrespondence": {429, 503}, + "ListClientReplies": {429, 503}, + "GetClientReply": {429, 503}, + "GetEverythingComments": {429, 503}, + "GetComment": {429, 503}, + "UpdateComment": {429, 503}, + "DeleteTool": {429, 503}, + "GetTool": {429, 503}, + "UpdateTool": {429, 503}, + "GetDocument": {429, 503}, + "UpdateDocument": {429, 503}, + "GetEverythingFiles": {429, 503}, + "GetEverythingForwards": {429, 503}, + "DestroyGaugeNeedle": {429, 503}, + "GetGaugeNeedle": {429, 503}, + "UpdateGaugeNeedle": {429, 503}, + "GetForward": {429, 503}, + "ListForwardReplies": {429, 503}, + "CreateForwardReply": {429, 503}, + "GetForwardReply": {429, 503}, + "GetInbox": {429, 503}, + "ListForwards": {429, 503}, + "ListLineupMarkers": {429, 503}, + "CreateLineupMarker": {429, 503}, + "DeleteLineupMarker": {429, 503}, + "UpdateLineupMarker": {429, 503}, + "GetMessageBoard": {429, 503}, + "ListMessages": {429, 503}, + "CreateMessage": {429, 503}, + "GetEverythingMessages": {429, 503}, + "GetMessage": {429, 503}, + "UpdateMessage": {429, 503}, + "GetMyAssignments": {429, 503}, + "GetMyCompletedAssignments": {429, 503}, + "GetMyDueAssignments": {429, 503}, + "GetMyPreferences": {429, 503}, + "UpdateMyPreferences": {429, 503}, + "GetMyProfile": {429, 503}, + "UpdateMyProfile": {429, 503}, + "GetQuestionReminders": {429, 503}, + "GetMyNotifications": {429, 503}, + "GetBubbleUps": {429, 503}, + "MarkAsRead": {429, 503}, + "ListPeople": {429, 503}, + "GetPerson": {429, 503}, + "DisableOutOfOffice": {429, 503}, + "GetOutOfOffice": {429, 503}, + "EnableOutOfOffice": {429, 503}, + "ListProjects": {429, 503}, + "CreateProject": {429, 503}, + "ListRecordings": {429, 503}, + "TrashProject": {429, 503}, + "GetProject": {429, 503}, + "UpdateProject": {429, 503}, + "ToggleGauge": {429, 503}, + "ListGaugeNeedles": {429, 503}, + "CreateGaugeNeedle": {429, 503}, + "ListProjectPeople": {429, 503}, + "UpdateProjectAccess": {429, 503}, + "GetProjectTimeline": {429, 503}, + "GetProjectTimesheet": {429, 503}, + "GetAnswer": {429, 503}, + "UpdateAnswer": {429, 503}, + "GetQuestionnaire": {429, 503}, + "ListQuestions": {429, 503}, + "CreateQuestion": {429, 503}, + "GetQuestion": {429, 503}, + "UpdateQuestion": {429, 503}, + "ListAnswers": {429, 503}, + "CreateAnswer": {429, 503}, + "ListQuestionAnswerers": {429, 503}, + "GetAnswersByPerson": {429, 503}, + "UpdateQuestionNotificationSettings": {429, 503}, + "ResumeQuestion": {429, 503}, + "PauseQuestion": {429, 503}, + "UnpinMessage": {429, 503}, + "PinMessage": {429, 503}, + "GetRecording": {429, 503}, + "ListRecordingBoosts": {429, 503}, + "CreateRecordingBoost": {429, 503}, + "SetClientVisibility": {429, 503}, + "ListComments": {429, 503}, + "CreateComment": {429, 503}, + "ListEvents": {429, 503}, + "ListEventBoosts": {429, 503}, + "CreateEventBoost": {429, 503}, + "UnarchiveRecording": {429, 503}, + "ArchiveRecording": {429, 503}, + "TrashRecording": {429, 503}, + "Unsubscribe": {429, 503}, + "GetSubscription": {429, 503}, + "Subscribe": {429, 503}, + "UpdateSubscription": {429, 503}, + "GetRecordingTimesheet": {429, 503}, + "CreateTimesheetEntry": {429, 503}, + "DisableTool": {429, 503}, + "EnableTool": {429, 503}, + "RepositionTool": {429, 503}, + "ListGauges": {429, 503}, + "GetProgressReport": {429, 503}, + "GetUpcomingSchedule": {429, 503}, + "GetTimesheetReport": {429, 503}, + "ListAssignablePeople": {429, 503}, + "GetAssignedTodos": {429, 503}, + "GetOverdueTodos": {429, 503}, + "GetPersonProgress": {429, 503}, + "GetScheduleEntry": {429, 503}, + "UpdateScheduleEntry": {429, 503}, + "GetScheduleEntryOccurrence": {429, 503}, + "GetSchedule": {429, 503}, + "UpdateScheduleSettings": {429, 503}, + "ListScheduleEntries": {429, 503}, + "CreateScheduleEntry": {429, 503}, + "Search": {429, 503}, + "GetSearchMetadata": {429, 503}, + "ListTemplates": {429, 503}, + "CreateTemplate": {429, 503}, + "DeleteTemplate": {429, 503}, + "GetTemplate": {429, 503}, + "UpdateTemplate": {429, 503}, + "CreateProjectFromTemplate": {429, 503}, + "GetProjectConstruction": {429, 503}, + "GetTimesheetEntry": {429, 503}, + "UpdateTimesheetEntry": {429, 503}, + "RepositionTodolistGroup": {429, 503}, + "GetTodolistOrGroup": {429, 503}, + "UpdateTodolistOrGroup": {429, 503}, + "ListTodolistGroups": {429, 503}, + "CreateTodolistGroup": {429, 503}, + "ListTodos": {429, 503}, + "CreateTodo": {429, 503}, + "GetEverythingCompletedTodos": {429, 503}, + "GetEverythingNoDueDateTodos": {429, 503}, + "GetEverythingOpenTodos": {429, 503}, + "GetEverythingOverdueTodos": {429, 503}, + "GetEverythingUnassignedTodos": {429, 503}, + "TrashTodo": {429, 503}, + "GetTodo": {429, 503}, + "ReplaceTodo": {429, 503}, + "UncompleteTodo": {429, 503}, + "CompleteTodo": {429, 503}, + "RepositionTodo": {429, 503}, + "RepositionTodolist": {429, 503}, + "GetTodoset": {429, 503}, + "GetHillChart": {429, 503}, + "UpdateHillChartSettings": {429, 503}, + "ListTodolists": {429, 503}, + "CreateTodolist": {429, 503}, + "GetUpload": {429, 503}, + "UpdateUpload": {429, 503}, + "ListUploadVersions": {429, 503}, + "GetVault": {429, 503}, + "UpdateVault": {429, 503}, + "ListDocuments": {429, 503}, + "CreateDocument": {429, 503}, + "ListUploads": {429, 503}, + "CreateUpload": {429, 503}, + "ListVaults": {429, 503}, + "CreateVault": {429, 503}, + "DeleteWebhook": {429, 503}, + "GetWebhook": {429, 503}, + "UpdateWebhook": {429, 503}, +} + +// GetOperationRetryOn returns the declared retryable status set for the given +// operation ID. ok is false when the operation carries no per-op set. +func GetOperationRetryOn(operationId string) ([]int, bool) { + r, ok := operationRetryOn[operationId] + return r, ok +} + // GetOperationRetryMax returns the per-operation retry ceiling (maximum total // attempts) for the given operation ID. ok is false when the operation carries // no per-op override, in which case the client-configured cap applies unchanged. diff --git a/go/templates/client.tmpl b/go/templates/client.tmpl index 215490aa9..b2b53f6f5 100644 --- a/go/templates/client.tmpl +++ b/go/templates/client.tmpl @@ -144,18 +144,32 @@ func WithLogger(logger *slog.Logger) ClientOption { } } -// isRetryableStatus returns true if the HTTP status code indicates a retryable error. -func isRetryableStatus(statusCode int) bool { - switch statusCode { - case http.StatusTooManyRequests, // 429 - http.StatusInternalServerError, // 500 - http.StatusBadGateway, // 502 - http.StatusServiceUnavailable, // 503 - http.StatusGatewayTimeout: // 504 - return true - default: - return false +// defaultRetryOn is behavior-model.json's declared default, applied only when an +// operation carries no x-basecamp-retry.retryOn — which cannot happen for a +// generated call site. +var defaultRetryOn = []int{http.StatusTooManyRequests, http.StatusServiceUnavailable} + +// isRetryableStatus reports whether the status is in the operation's DECLARED +// retry_on set. The set is per-operation and exhaustive: a status the spec never +// declared retryable — including 500, 502, and 504 — is not retried, and is +// surfaced to the caller on the first attempt. ParseHTTPError still classifies +// those statuses as retryable errors for the caller's benefit; that is a +// caller-facing hint and deliberately does not widen this gate. +func isRetryableStatus(statusCode int, operationId string) bool { + // Only an ABSENT entry falls back. A present-but-empty set is an operation + // declaring "never retry on any status", which is not the same as declaring + // nothing — collapsing the two would silently re-enable 429/503 retries on an + // operation that opted out. + retryOn, ok := operationRetryOn[operationId] + if !ok { + retryOn = defaultRetryOn + } + for _, status := range retryOn { + if status == statusCode { + return true + } } + return false } // captureReplayBody inspects the finalized first-attempt request and returns a @@ -414,7 +428,7 @@ func (c *{{ $clientTypeName }}) doWithRetry(ctx context.Context, buildRequest fu } // Success or non-retryable status - if !isRetryableStatus(resp.StatusCode) { + if !isRetryableStatus(resp.StatusCode, operationId) { return resp, nil } @@ -818,6 +832,37 @@ var operationRetryMax = map[string]int{ {{end}} } +// operationRetryOn maps operation IDs to their declared retryable status set +// from the x-basecamp-retry extension. Like operationRetryMax it is a separate +// map rather than a field on OperationMetadata, so wiring it up does not change +// that exported struct's shape. +var operationRetryOn = map[string][]int{ +{{range . -}} +{{$opid := .OperationId -}} +{{$retryOn := "" -}} +{{if .Spec -}} +{{if .Spec.Extensions -}} +{{$retry := index .Spec.Extensions "x-basecamp-retry" -}} +{{if $retry -}} +{{if index $retry "retryOn" -}} +{{range $i, $status := index $retry "retryOn" -}} +{{if $i}}{{$retryOn = printf "%s, %v" $retryOn $status}}{{else}}{{$retryOn = printf "%v" $status}}{{end -}} +{{end -}} +{{end -}} +{{end -}} +{{end -}} +{{end -}} + "{{$opid}}": { {{$retryOn}} }, +{{end}} +} + +// GetOperationRetryOn returns the declared retryable status set for the given +// operation ID. ok is false when the operation carries no per-op set. +func GetOperationRetryOn(operationId string) ([]int, bool) { + r, ok := operationRetryOn[operationId] + return r, ok +} + // GetOperationRetryMax returns the per-operation retry ceiling (maximum total // attempts) for the given operation ID. ok is false when the operation carries // no per-op override, in which case the client-configured cap applies unchanged. diff --git a/python/src/basecamp/_async_http.py b/python/src/basecamp/_async_http.py index 3f430e551..2797bf8a4 100644 --- a/python/src/basecamp/_async_http.py +++ b/python/src/basecamp/_async_http.py @@ -174,7 +174,7 @@ async def _request_with_retry( allow_cross_origin=allow_cross_origin, ) except (RateLimitError, NetworkError, ApiError) as e: - if not e.retryable: + if not self._is_retryable_error(e, operation): raise last_error = e if attempt >= max_attempts: @@ -311,6 +311,41 @@ def _is_retryable_operation(self, operation: str) -> bool: op_meta = self._metadata.get(operation, {}) return op_meta.get("idempotent", False) + # behavior-model.json's declared default, used when an operation carries no + # retry_on of its own. + DEFAULT_RETRY_ON = frozenset({429, 503}) + + def _is_retryable_error(self, error: BasecampError, operation: str | None) -> bool: + # A network error carries no HTTP status, so the declared status set does + # not apply; SPEC section 7's network-error rule governs, and errors.py's + # classification is the right signal there. + if error.http_status is None: + return error.retryable + # No operation id means no behavior-model metadata — today only the + # Launchpad authorization GET issued by get_absolute(). Non-Smithy + # traffic keeps its pre-Smithy retry contract; applying the generated + # gate here would impose API policy on OAuth authorization traffic. + if operation is None: + return error.retryable + # For a governed operation the DECLARED set is authoritative in BOTH + # directions. It must not be widened by errors.py (which marks + # 500/502/503/504 retryable for the caller's benefit), and it must not be + # vetoed by it either: if an operation declares a status retryable, a + # future errors.py classifying that status non-retryable must not + # silently disable the declared retry. + return error.http_status in self._operation_retry_on(operation) + + def _operation_retry_on(self, operation: str) -> frozenset[int]: + retry = (self._metadata.get(operation) or {}).get("retry") or {} + statuses = retry.get("retry_on") + # `is None`, not truthiness: an operation that declares `retry_on: []` + # means "never retry on any status", which is not the same as declaring + # nothing. Collapsing the two would silently re-enable 429/503 retries on + # an operation that opted out. + if statuses is None: + return self.DEFAULT_RETRY_ON + return frozenset(statuses) + def _apply_operation_retry_max(self, operation: str | None, max_attempts: int) -> int: # Apply the per-operation retry ceiling from metadata as an upper bound on # attempts: effective = min(client cap, operation's retry.max). The diff --git a/python/src/basecamp/_http.py b/python/src/basecamp/_http.py index cb5f1fa9e..771ed8180 100644 --- a/python/src/basecamp/_http.py +++ b/python/src/basecamp/_http.py @@ -174,7 +174,7 @@ def _request_with_retry( allow_cross_origin=allow_cross_origin, ) except (RateLimitError, NetworkError, ApiError) as e: - if not e.retryable: + if not self._is_retryable_error(e, operation): raise last_error = e if attempt >= max_attempts: @@ -311,6 +311,41 @@ def _is_retryable_operation(self, operation: str) -> bool: op_meta = self._metadata.get(operation, {}) return op_meta.get("idempotent", False) + # behavior-model.json's declared default, used when an operation carries no + # retry_on of its own. + DEFAULT_RETRY_ON = frozenset({429, 503}) + + def _is_retryable_error(self, error: BasecampError, operation: str | None) -> bool: + # A network error carries no HTTP status, so the declared status set does + # not apply; SPEC section 7's network-error rule governs, and errors.py's + # classification is the right signal there. + if error.http_status is None: + return error.retryable + # No operation id means no behavior-model metadata — today only the + # Launchpad authorization GET issued by get_absolute(). Non-Smithy + # traffic keeps its pre-Smithy retry contract; applying the generated + # gate here would impose API policy on OAuth authorization traffic. + if operation is None: + return error.retryable + # For a governed operation the DECLARED set is authoritative in BOTH + # directions. It must not be widened by errors.py (which marks + # 500/502/503/504 retryable for the caller's benefit), and it must not be + # vetoed by it either: if an operation declares a status retryable, a + # future errors.py classifying that status non-retryable must not + # silently disable the declared retry. + return error.http_status in self._operation_retry_on(operation) + + def _operation_retry_on(self, operation: str) -> frozenset[int]: + retry = (self._metadata.get(operation) or {}).get("retry") or {} + statuses = retry.get("retry_on") + # `is None`, not truthiness: an operation that declares `retry_on: []` + # means "never retry on any status", which is not the same as declaring + # nothing. Collapsing the two would silently re-enable 429/503 retries on + # an operation that opted out. + if statuses is None: + return self.DEFAULT_RETRY_ON + return frozenset(statuses) + def _apply_operation_retry_max(self, operation: str | None, max_attempts: int) -> int: # Apply the per-operation retry ceiling from metadata as an upper bound on # attempts: effective = min(client cap, operation's retry.max). The diff --git a/python/tests/test_http.py b/python/tests/test_http.py index 0d88822b0..82f52bfa0 100644 --- a/python/tests/test_http.py +++ b/python/tests/test_http.py @@ -14,6 +14,7 @@ from basecamp.errors import ( ApiError, AuthError, + BasecampError, NetworkError, NotFoundError, RateLimitError, @@ -117,7 +118,11 @@ def test_get_retries_on_503(self): assert route.call_count == 2 @respx.mock - def test_get_retries_on_500(self): + def test_get_without_operation_still_retries_500(self): + # No operation id means no behavior-model metadata, so the declared + # retry_on gate does not apply and the historical contract stands. + # Generated reads always pass an operation; see + # TestDeclaredRetryStatuses for the governed path. route = respx.get("https://3.basecampapi.com/test") route.side_effect = [ httpx.Response(500), @@ -524,3 +529,171 @@ async def test_async_mutation_respects_op_ceiling(self): await client.for_account("999").account.update_account_name(name="x") assert route.call_count == 2 await client.close() + + +# behavior-model.json declares retry_on: [429, 503] for all 226 operations. +# These fixtures mirror that so the tests do not depend on any single real op. +STATUS_GATE_METADATA = { + "GovernedOp": {"idempotent": True, "retry": {"max": 3, "retry_on": [429, 503]}}, +} + + +class TestDeclaredRetryStatuses: + """Gate 3's status set. A status outside the operation's declared retry_on — + including 500, 502 and 504 — is surfaced on the first attempt. errors.py + still reports those as retryable to callers; that classification is + deliberately not the transport's gate.""" + + @pytest.mark.parametrize( + "status,want_attempts", + [(429, 3), (503, 3), (500, 1), (502, 1), (504, 1)], + ) + @respx.mock + def test_sync_mutation_status_gate(self, status, want_attempts): + route = respx.put("https://3.basecampapi.com/thing").mock( + return_value=httpx.Response(status, headers={"Retry-After": "0"}) + ) + client = make_client_meta(STATUS_GATE_METADATA) + with pytest.raises((ApiError, RateLimitError)): + client.put("/thing", json_body={"x": 1}, operation="GovernedOp") + assert route.call_count == want_attempts + + @pytest.mark.parametrize( + "status,want_attempts", + [(429, 3), (503, 3), (500, 1), (502, 1), (504, 1)], + ) + @pytest.mark.asyncio + @respx.mock + async def test_async_mutation_status_gate(self, status, want_attempts): + route = respx.put("https://3.basecampapi.com/thing").mock( + return_value=httpx.Response(status, headers={"Retry-After": "0"}) + ) + client = make_async_client_meta(STATUS_GATE_METADATA) + with pytest.raises((ApiError, RateLimitError)): + await client.put("/thing", json_body={"x": 1}, operation="GovernedOp") + assert route.call_count == want_attempts + + @respx.mock + def test_sync_read_status_gate(self): + # Generated reads pass an operation id (threaded by _base.py), so the + # read path is governed too. + route = respx.get("https://3.basecampapi.com/thing").mock(return_value=httpx.Response(500)) + client = make_client_meta(STATUS_GATE_METADATA) + with pytest.raises(ApiError): + client.get("/thing", operation="GovernedOp") + assert route.call_count == 1 + + @pytest.mark.asyncio + @respx.mock + async def test_async_read_status_gate(self): + route = respx.get("https://3.basecampapi.com/thing").mock(return_value=httpx.Response(500)) + client = make_async_client_meta(STATUS_GATE_METADATA) + with pytest.raises(ApiError): + await client.get("/thing", operation="GovernedOp") + assert route.call_count == 1 + + +class TestAuthorizationRetryContractUnchanged: + """Launchpad authorization goes through get_absolute(), which shares the + retry loop with generated traffic but is NOT a Smithy operation — it passes + no operation id and carries no behavior-model metadata. The declared status + gate and the per-operation ceiling must not reach it. These fail if + generated policy ever leaks onto OAuth traffic.""" + + @respx.mock + def test_sync_authorization_still_retries_500(self): + route = respx.get("https://launchpad.37signals.com/authorization.json") + route.side_effect = [ + httpx.Response(500), + httpx.Response(200, json={"ok": True}), + ] + client = make_client_meta(STATUS_GATE_METADATA) + resp = client.get_absolute("https://launchpad.37signals.com/authorization.json") + assert resp.status_code == 200 + assert route.call_count == 2 + + @pytest.mark.asyncio + @respx.mock + async def test_async_authorization_still_retries_500(self): + route = respx.get("https://launchpad.37signals.com/authorization.json") + route.side_effect = [ + httpx.Response(500), + httpx.Response(200, json={"ok": True}), + ] + client = make_async_client_meta(STATUS_GATE_METADATA) + resp = await client.get_absolute("https://launchpad.37signals.com/authorization.json") + assert resp.status_code == 200 + assert route.call_count == 2 + + @respx.mock + def test_sync_authorization_uses_full_configured_attempts(self): + # No operation ceiling applies either, so all 5 configured attempts run. + route = respx.get("https://launchpad.37signals.com/authorization.json").mock(return_value=httpx.Response(503)) + client = make_client_meta(STATUS_GATE_METADATA, max_retries=5) + with pytest.raises(ApiError): + client.get_absolute("https://launchpad.37signals.com/authorization.json") + assert route.call_count == 5 + + @pytest.mark.asyncio + @respx.mock + async def test_async_authorization_uses_full_configured_attempts(self): + route = respx.get("https://launchpad.37signals.com/authorization.json").mock(return_value=httpx.Response(503)) + client = make_async_client_meta(STATUS_GATE_METADATA, max_retries=5) + with pytest.raises(ApiError): + await client.get_absolute("https://launchpad.37signals.com/authorization.json") + assert route.call_count == 5 + + +class TestDeclaredSetIsAuthoritative: + """The declared retry_on set governs a Smithy operation in BOTH directions. + + errors.py classifies statuses for the CALLER (it marks 500/502/503/504 + retryable). That classification must neither widen the transport's gate nor + veto it: if an operation declares a status retryable, the transport retries + it even if errors.py would not have.""" + + @respx.mock + def test_declared_status_retries_even_when_classified_non_retryable(self): + # 409 is not retryable per errors.py, but this operation declares it. + metadata = {"OptInOp": {"idempotent": True, "retry": {"max": 3, "retry_on": [409]}}} + route = respx.put("https://3.basecampapi.com/thing").mock(return_value=httpx.Response(409)) + client = make_client_meta(metadata) + with pytest.raises(BasecampError): + client.put("/thing", json_body={"x": 1}, operation="OptInOp") + assert route.call_count == 3, "a declared status must retry regardless of errors.py" + + @respx.mock + def test_explicitly_empty_retry_on_never_retries(self): + # `retry_on: []` means "never retry on any status" — distinct from + # declaring nothing, which falls back to the [429, 503] default. + metadata = {"OptOutOp": {"idempotent": True, "retry": {"max": 3, "retry_on": []}}} + route = respx.put("https://3.basecampapi.com/thing").mock( + return_value=httpx.Response(503, headers={"Retry-After": "0"}) + ) + client = make_client_meta(metadata) + with pytest.raises(ApiError): + client.put("/thing", json_body={"x": 1}, operation="OptOutOp") + assert route.call_count == 1, "an explicitly empty retry_on must not fall back to the default" + + @respx.mock + def test_absent_retry_on_falls_back_to_the_default(self): + metadata = {"NoBlockOp": {"idempotent": True, "retry": {"max": 3}}} + route = respx.put("https://3.basecampapi.com/thing").mock( + return_value=httpx.Response(503, headers={"Retry-After": "0"}) + ) + client = make_client_meta(metadata) + with pytest.raises(ApiError): + client.put("/thing", json_body={"x": 1}, operation="NoBlockOp") + assert route.call_count == 3, "no declared set means the [429, 503] default applies" + + @pytest.mark.asyncio + @respx.mock + async def test_async_explicitly_empty_retry_on_never_retries(self): + metadata = {"OptOutOp": {"idempotent": True, "retry": {"max": 3, "retry_on": []}}} + route = respx.put("https://3.basecampapi.com/thing").mock( + return_value=httpx.Response(503, headers={"Retry-After": "0"}) + ) + client = make_async_client_meta(metadata) + with pytest.raises(ApiError): + await client.put("/thing", json_body={"x": 1}, operation="OptOutOp") + assert route.call_count == 1 diff --git a/scripts/check-retry-metadata-parity.py b/scripts/check-retry-metadata-parity.py index 11f7bb7c7..f244b1c94 100755 --- a/scripts/check-retry-metadata-parity.py +++ b/scripts/check-retry-metadata-parity.py @@ -21,8 +21,8 @@ - typescript/src/generated/metadata.ts (camelCase) - kotlin/.../generated/Metadata.kt (positional) - swift/Sources/Basecamp/Generated/Metadata.swift (labelled) - * max only: - - go/pkg/generated/client.gen.go operationRetryMax map + * max + retry_on: + - go/pkg/generated/client.gen.go operationRetryMax + operationRetryOn maps (2) Runtime consumption (TOKEN-SMOKE classification, NOT behavioral proof). Which emitted fields are read at runtime, per SDK, checked by the presence @@ -33,8 +33,9 @@ Fields emitted but never read are guarded for PARITY only (criterion 1) and classified emitted-but-runtime-inert — NOT claimed as runtime parity: * TypeScript / Swift / Kotlin: consume the full tuple. - * Go / Python: consume only `max` (per-op ceiling); the - other fields are emitted-but-inert. + * Go / Python: consume `max` (per-op ceiling) AND + `retry_on` (the status gate); base_delay + and backoff are emitted-but-inert. * Ruby: consumes NONE (GET-only retry); every emitted retry field is inert. @@ -163,6 +164,21 @@ def from_go_max() -> dict[str, int]: return {m.group("op"): int(m.group("max")) for m in pat.finditer(block.group(1))} +def from_go_retry_on() -> dict[str, tuple]: + # Go emits the declared retryable status set as `operationRetryOn`, a + # sibling of operationRetryMax and likewise kept off the exported + # OperationMetadata struct. + text = (ROOT / "go/pkg/generated/client.gen.go").read_text() + block = re.search(r"var operationRetryOn = map\[string\]\[\]int\{(.*?)\n\}", text, re.DOTALL) + if not block: + raise ValueError("operationRetryOn map not found in client.gen.go") + pat = re.compile(r'"(?P\w+)":\s*\{(?P[0-9,\s]*)\},') + return { + m.group("op"): tuple(int(x) for x in re.findall(r"\d+", m.group("ro"))) + for m in pat.finditer(block.group(1)) + } + + # --- checks ------------------------------------------------------------------ @@ -224,18 +240,39 @@ def check_max_only(name: str, emitted: dict[str, int], model: dict[str, tuple]) ("Kotlin", "full tuple", "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/http/BasecampHttpClient.kt", ["opRetry.retryOn", "opRetry?.maxRetries", "opRetry?.baseDelayMs"], [], "BasecampHttpClient.kt: status in opRetry.retryOn, opRetry.maxRetries/baseDelayMs"), - ("Go", "max only", "go/templates/client.tmpl", - ["opMax < maxAttempts", "operationRetryMax[operationId]"], ["RetryBaseDelayMs", "RetryOn"], - "doWithRetry applies min(client cap, operationRetryMax); base_delay/retry_on/backoff emitted-but-inert"), - ("Python", "max only", "python/src/basecamp/_http.py", - ['.get("retry", {}).get("max")'], [], - "_request_with_retry applies min(client cap, retry.max); other fields emitted-but-inert"), + ("Go", "max + retry_on", "go/templates/client.tmpl", + ["opMax < maxAttempts", "operationRetryMax[operationId]", + "operationRetryOn[operationId]", "isRetryableStatus(resp.StatusCode, operationId)"], + ["RetryBaseDelayMs"], + "doWithRetry applies min(client cap, operationRetryMax) and gates status retry on operationRetryOn; base_delay/backoff emitted-but-inert"), + ("Python", "max + retry_on", "python/src/basecamp/_http.py", + ['.get("retry", {}).get("max")', 'retry.get("retry_on")', "_is_retryable_error"], [], + "_request_with_retry applies min(client cap, retry.max) and gates status retry on the declared retry_on; other fields emitted-but-inert"), ("Ruby", "none", "ruby/lib/basecamp/http.rb", [], ["maxAttempts", "maxRetries"], "http.rb retries GET only; every emitted per-op retry field is inert"), ] +def check_retry_on_only(name: str, emitted: dict[str, tuple], model: dict[str, tuple]) -> int: + errors = 0 + missing = sorted(set(model) - set(emitted)) + extra = sorted(set(emitted) - set(model)) + if missing: + fail(f"{name}: {len(missing)} operation(s) missing a retryOn set, e.g. {missing[:3]}") + errors += len(missing) + if extra: + fail(f"{name}: {len(extra)} operation(s) not in behavior-model.json, e.g. {extra[:3]}") + errors += len(extra) + for op in sorted(set(model) & set(emitted)): + if emitted[op] != model[op][3]: + fail(f"{name}: {op} retryOn {emitted[op]} != model {model[op][3]}") + errors += 1 + if errors == 0: + print(f" ✓ {name}: {len(emitted)} operations match behavior-model.json (retry_on)") + return errors + + def check_runtime_consumption() -> int: errors = 0 for sdk, level, rel, required, forbidden, note in RUNTIME_CONSUMPTION: @@ -265,6 +302,7 @@ def main() -> int: errors += check_full_tuple("Kotlin Metadata.kt", from_kotlin(), model) errors += check_full_tuple("Swift Metadata.swift", from_swift(), model) errors += check_max_only("Go operationRetryMax", from_go_max(), model) + errors += check_retry_on_only("Go operationRetryOn", from_go_retry_on(), model) print("\nCriterion 2 — runtime consumption (token-smoke classification, not behavioral proof):") errors += check_runtime_consumption()