diff --git a/SPEC.md b/SPEC.md index eb3983992..08140a729 100644 --- a/SPEC.md +++ b/SPEC.md @@ -402,8 +402,8 @@ The error must be retryable. Two categories qualify: ### Cross-SDK Divergence `[CONFLICT]` - **TypeScript** implements the three-gate algorithm but chains at most 1 retry — on a retryable status, TS returns `fetch(retryRequest)` which bypasses middleware after the first retry (waiver 2B.1 in `rubric-audit.json`). **Kotlin** implements the three-gate algorithm for HTTP status retries (POST retries only when `idempotent: true`, full exponential backoff) but does not retry on network errors — transport exceptions are returned immediately as `BasecampException.Network`. -- **Go** is stricter: only GET retries with exponential backoff; all non-GET methods make a single attempt (plus one re-attempt after successful 401 token refresh). No idempotency gate. -- **Ruby** is stricter: only GET retries; all non-GET methods do not retry. Go and Ruby are acceptably conservative. +- **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. 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 three idempotent POSTs (`CompleteTodo`, `PauseQuestion`, `SubscribeToCardColumn`) keep retrying. The gate covers both retry paths — HTTP status (`429`/`503`) and network errors — so Swift retries network errors (unlike Kotlin/TS) but only for retry-eligible operations. `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. **TS note:** TS retry returns `fetch(retryRequest)` which bypasses middleware after the first retry, so TS effectively caps at 1 retry per request regardless of `max_attempts`. This is a known limitation (waiver 2B.1). @@ -1563,7 +1563,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. 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 | Simplified: only GET retries with exponential backoff. All non-GET methods do not retry (single attempt, plus one re-attempt after successful 401 token refresh). | +| 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`. | | 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. | diff --git a/conformance/runner/go/main.go b/conformance/runner/go/main.go index 9d7399ec7..c62ac9150 100644 --- a/conformance/runner/go/main.go +++ b/conformance/runner/go/main.go @@ -477,6 +477,11 @@ func executeOperation(ctx context.Context, account *basecamp.AccountClient, tc T _, err := account.Todos().Create(ctx, todolistID, &basecamp.CreateTodoRequest{Content: content}) return operationResult{err: err} + case "CompleteTodo": + todoID := getInt64Param(tc.PathParams, "todoId") + err := account.Todos().Complete(ctx, todoID) + return operationResult{err: err} + case "UpdateTodo": todoID := getInt64Param(tc.PathParams, "todoId") req := &basecamp.UpdateTodoRequest{ diff --git a/conformance/runner/python/runner.py b/conformance/runner/python/runner.py index fd101b162..ac5cde72b 100644 --- a/conformance/runner/python/runner.py +++ b/conformance/runner/python/runner.py @@ -94,6 +94,8 @@ def __call__(self, operation: str, *, path_params: dict, query_params: dict, bod return self._account.todos.get(todo_id=path_params["todoId"]) case "CreateTodo": return self._account.todos.create(todolist_id=path_params["todolistId"], content=body["content"]) + case "CompleteTodo": + return self._account.todos.complete(todo_id=path_params["todoId"]) case "UpdateTodo": return self._account.todos.update( todo_id=path_params["todoId"], diff --git a/conformance/runner/ruby/runner.rb b/conformance/runner/ruby/runner.rb index 1010b6b96..453d6f295 100644 --- a/conformance/runner/ruby/runner.rb +++ b/conformance/runner/ruby/runner.rb @@ -197,6 +197,7 @@ def todo_write_kwargs(body) RUBY_SKIPS = Set.new([ "PUT operation is naturally idempotent", "DELETE operation is naturally idempotent", + "POST operation retries when marked idempotent", "Total count header is accessible", "Missing X-Total-Count returns zero", "Pagination stops at maxPages safety cap", @@ -209,6 +210,7 @@ def todo_write_kwargs(body) RUBY_SKIP_REASONS = { "PUT operation is naturally idempotent" => "Ruby SDK only retries GET", "DELETE operation is naturally idempotent" => "Ruby SDK only retries GET", + "POST operation retries when marked idempotent" => "Ruby SDK only retries GET", "Total count header is accessible" => "Ruby SDK paginate doesn't expose X-Total-Count metadata", "Missing X-Total-Count returns zero" => "Ruby SDK paginate doesn't expose X-Total-Count metadata", "Pagination stops at maxPages safety cap" => "Ruby SDK paginate doesn't expose truncation metadata", diff --git a/conformance/runner/typescript/runner.test.ts b/conformance/runner/typescript/runner.test.ts index a743f4e01..3002b5d7c 100644 --- a/conformance/runner/typescript/runner.test.ts +++ b/conformance/runner/typescript/runner.test.ts @@ -199,6 +199,10 @@ async function executeOperation( }); break; + case "CompleteTodo": + await client.todos.complete(Number(params.todoId)); + break; + case "UpdateTodo": // Merge-safe update: GET then full PUT; only fixture-present keys are passed. await client.todos.update(Number(params.todoId), mapTodoWireFields(body)); diff --git a/conformance/tests/idempotency.json b/conformance/tests/idempotency.json index a6582d552..5deb54d71 100644 --- a/conformance/tests/idempotency.json +++ b/conformance/tests/idempotency.json @@ -50,5 +50,22 @@ {"type": "statusCode", "expected": 503} ], "tags": ["no-retry", "post"] + }, + { + "name": "POST operation retries when marked idempotent", + "description": "An idempotent-flagged POST (x-basecamp-idempotent / behavior-model idempotent:true) must retry on 503, unlike a non-idempotent POST. Pins the metadata-driven three-gate liveness that distinguishes Go/TS/Kotlin/Python from Ruby's GET-only retry.", + "operation": "CompleteTodo", + "method": "POST", + "path": "/todos/{todoId}/completion.json", + "pathParams": {"todoId": 456}, + "mockResponses": [ + {"status": 503}, + {"status": 204} + ], + "assertions": [ + {"type": "requestCount", "expected": 2}, + {"type": "noError"} + ], + "tags": ["idempotent", "post"] } ] diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt index b28b461fb..d29b19d10 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt @@ -705,6 +705,11 @@ private suspend fun dispatchOperation(tc: TestCase, account: AccountClient): Dis DispatchResult() } + "CompleteTodo" -> { + account.todos.complete(tc.pathParams.longParam("todoId")) + DispatchResult() + } + "GetTimesheetEntry" -> { val entryId = tc.pathParams.longParam("timesheetEntryId") .let { if (it != 0L) it else tc.pathParams.longParam("entryId") }