From 2dfd8d15e06bb1a05d7d7d6c701bc97e3047d143 Mon Sep 17 00:00:00 2001 From: Juha Itkonen Date: Wed, 15 Jul 2026 00:30:56 +0300 Subject: [PATCH] Fix Codex OAuth rate limit reset handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the authoritative usage-limit response reset and bound structured rate-limit waits so quota snapshot metadata cannot stall sessions for hours. 🤖 Generated with [ECA](https://eca.dev) (openai/gpt-5.6-sol - xhigh) Co-Authored-By: eca-agent --- CHANGELOG.md | 1 + docs/config.json | 6 +- src/eca/llm_api.clj | 22 ++++--- src/eca/llm_providers/errors.clj | 87 +++++++++++++++----------- test/eca/llm_api_test.clj | 54 ++++++++++++++-- test/eca/llm_providers/errors_test.clj | 80 +++++++++++++++++------ test/eca/llm_providers/openai_test.clj | 21 +++++++ 7 files changed, 203 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17f8b96cc..6345071cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Fix ChatGPT/Codex OAuth rate-limit retries using the response reset instead of quota snapshot headers; cap waits at 60 seconds by default. - Fix prompt cache invalidation warning after clearing the chat and changing model. #530 - Fix missing line break after "Prompt stopped" message when followed by another system message. diff --git a/docs/config.json b/docs/config.json index 0b4a76a39..0c842a7db 100644 --- a/docs/config.json +++ b/docs/config.json @@ -696,8 +696,10 @@ }, "rateLimitMaxWaitSeconds": { "type": "integer", - "description": "Optional cap on how long to wait for a rate limit reset (derived from provider response headers like retry-after). Unset: ECA sleeps until the reset time and auto-resumes, showing the reset time in chat. When set and the reset is further away, ECA gives up retrying and surfaces the reset time in the error.", - "markdownDescription": "Optional cap on how long to wait for a rate limit reset (derived from provider response headers like `retry-after`). Unset: ECA sleeps until the reset time and auto-resumes, showing the reset time in chat. When set and the reset is further away, ECA gives up retrying and surfaces the reset time in the error." + "minimum": 0, + "default": 60, + "description": "Maximum total delay before retrying a provider rate limit, including ECA's one-second safety buffer. The delay is derived from retry-after or provider-specific reset data and defaults to a 60-second cap. When the total delay exceeds the cap, ECA gives up retrying and surfaces the reset time in the error.", + "markdownDescription": "Maximum total delay before retrying a provider rate limit, including ECA's one-second safety buffer. The delay is derived from `retry-after` or provider-specific reset data and defaults to a 60-second cap. When the total delay exceeds the cap, ECA gives up retrying and surfaces the reset time in the error." }, "models": { "type": "object", diff --git a/src/eca/llm_api.clj b/src/eca/llm_api.clj index caa87c660..5dc4dbede 100644 --- a/src/eca/llm_api.clj +++ b/src/eca/llm_api.clj @@ -39,6 +39,7 @@ (def ^:private default-base-delay-ms 2000) (def ^:private default-backoff-multiplier 2) (def ^:private max-delay-ms 60000) +(def ^:private default-rate-limit-max-wait-seconds 60) (def ^:private rate-limit-wait-buffer-ms 1000) (def ^:private cancel-check-interval-ms 100) @@ -508,22 +509,27 @@ default-max-retries) rl-wait (when (= :rate-limited error-type) (llm-providers.errors/rate-limit-wait (:headers error-data) + (:body error-data) (System/currentTimeMillis))) - max-wait-ms (some-> (:rateLimitMaxWaitSeconds provider-config) long (* 1000)) - wait-too-long? (boolean (and rl-wait - max-wait-ms - (> (long (:delay-ms rl-wait)) (long max-wait-ms))))] + rate-limit-delay-ms (some-> rl-wait + :delay-ms + long + (+ rate-limit-wait-buffer-ms)) + max-wait-ms (* 1000 + (long (or (:rateLimitMaxWaitSeconds provider-config) + default-rate-limit-max-wait-seconds))) + wait-too-long? (boolean (and rate-limit-delay-ms + (> rate-limit-delay-ms max-wait-ms)))] (if (and (contains? #{:rate-limited :overloaded :retryable-custom :premature-stop} error-type) (< attempt max-retries) (not wait-too-long?) (not (cancelled?))) - (let [delay-ms (if rl-wait - (+ (long (:delay-ms rl-wait)) rate-limit-wait-buffer-ms) - (retry-delay-ms attempt))] + (let [delay-ms (or rate-limit-delay-ms + (retry-delay-ms attempt))] (logger/info logger-tag (format "Retryable error (attempt %d/%d), retrying in %ds%s" (inc attempt) max-retries (quot delay-ms 1000) - (if rl-wait " (rate limit reset from headers)" "")) + (if rl-wait " (rate limit reset from provider response)" "")) {:error-type error-type :status (:status error-data)}) (when on-retry diff --git a/src/eca/llm_providers/errors.clj b/src/eca/llm_providers/errors.clj index cf0956fd1..5f3aee8fb 100644 --- a/src/eca/llm_providers/errors.clj +++ b/src/eca/llm_providers/errors.clj @@ -4,6 +4,7 @@ Supports context overflow, rate limiting, authentication, and overload detection across multiple providers (Anthropic, OpenAI, Google, Ollama, etc.)." (:require + [cheshire.core :as json] [clojure.string :as string])) (set! *warn-on-reflection* true) @@ -203,23 +204,30 @@ (reduce +) long))) -(defn ^:private codex-window-reset-ms - "ChatGPT Codex quota headers (`x-codex--*`): latest reset epoch-ms - among exhausted windows (used-percent >= 100), nil otherwise. Windows with - headroom are ignored on purpose: these headers come on every response with - far-future resets, so only a clearly exhausted window is a wait signal." - [headers now-ms] - (some->> ["primary" "secondary"] - (keep (fn [w] - (let [used (some-> (header-str headers (str "x-codex-" w "-used-percent")) - parse-double) - reset-at (epoch-str->ms (header-str headers (str "x-codex-" w "-reset-at"))) - reset-after-s (parse-long-str (header-str headers (str "x-codex-" w "-reset-after-seconds")))] - (when (and used (>= used 100.0)) - (or reset-at - (some-> reset-after-s (* 1000) (+ (long now-ms)))))))) - seq - (apply max))) +(defn ^:private response-field [m field] + (when (map? m) + (or (get m field) + (get m (keyword field))))) + +(defn ^:private codex-usage-limit-reset-ms + "ChatGPT Codex OAuth usage-limit reset from the 429 JSON response body." + [body] + (let [body (cond + (map? body) body + (string? body) (try + (json/parse-string body) + (catch Exception _ nil)) + :else nil) + error (response-field body "error") + error-type (response-field error "type") + reset-at (response-field error "resets_at")] + (when (and (= "usage_limit_reached" error-type) + (integer? reset-at)) + (* (long reset-at) 1000)))) + +(defn ^:private future-reset-ms [reset-ms now-ms] + (when (and reset-ms (> (long reset-ms) (long now-ms))) + (long reset-ms))) (defn ^:private bucket-resets "All rate-limit reset headers (`*ratelimit*-reset` like Anthropic's or @@ -240,29 +248,36 @@ headers)) (defn rate-limit-wait - "Computes when a rate-limited request can be retried, from HTTP response - headers (lowercase string keys, as returned by the http client). + "Computes when a rate-limited request can be retried. + + The two-argument arity reads HTTP response headers. The three-argument arity + additionally reads the response body used by ChatGPT Codex OAuth errors. Precedence: 1. `retry-after` (delta seconds or HTTP-date) - 2. `anthropic-ratelimit-unified-reset` (epoch seconds; subscription session limits) - 3. `x-codex--reset-at/-reset-after-seconds` (ChatGPT subscription), - only for exhausted windows (used-percent >= 100) + 2. ChatGPT Codex `usage_limit_reached` body `error.resets_at` (epoch seconds) + 3. `anthropic-ratelimit-unified-reset` (epoch seconds; subscription session limits) 4. rate-limit reset buckets (Anthropic `*ratelimit*-reset` RFC 3339/epoch, OpenAI `*ratelimit*-reset-` Go-style durations): latest exhausted bucket (remaining = 0), else earliest future reset. - Returns {:delay-ms N :resets-at epoch-ms} or nil when headers give no usable - future reset (callers should fall back to exponential backoff)." - [headers now-ms] - (when (map? headers) - (let [resets-at (or (retry-after-ms headers now-ms) - (epoch-str->ms (header-str headers "anthropic-ratelimit-unified-reset")) - (codex-window-reset-ms headers now-ms) - (let [buckets (filter #(> (long (:reset-ms %)) (long now-ms)) (bucket-resets headers now-ms)) - exhausted (filter #(some-> (:remaining %) (<= 0)) buckets)] - (or (some->> (seq exhausted) (map :reset-ms) (apply max)) - (some->> (seq buckets) (map :reset-ms) (apply min)))))] - (when (and resets-at (> (long resets-at) (long now-ms))) - {:delay-ms (- (long resets-at) (long now-ms)) - :resets-at (long resets-at)})))) + Codex quota-window headers are status snapshots, not retry instructions. + + Returns {:delay-ms N :resets-at epoch-ms} or nil when no usable future reset + is present (callers should fall back to exponential backoff)." + ([headers now-ms] + (rate-limit-wait headers nil now-ms)) + ([headers body now-ms] + (let [headers (if (map? headers) headers {}) + buckets (filter #(> (long (:reset-ms %)) (long now-ms)) (bucket-resets headers now-ms)) + exhausted (filter #(some-> (:remaining %) (<= 0)) buckets) + bucket-reset (or (some->> (seq exhausted) (map :reset-ms) (apply max)) + (some->> (seq buckets) (map :reset-ms) (apply min))) + resets-at (some #(future-reset-ms % now-ms) + [(retry-after-ms headers now-ms) + (codex-usage-limit-reset-ms body) + (epoch-str->ms (header-str headers "anthropic-ratelimit-unified-reset")) + bucket-reset])] + (when resets-at + {:delay-ms (- resets-at (long now-ms)) + :resets-at resets-at})))) diff --git a/test/eca/llm_api_test.clj b/test/eca/llm_api_test.clj index ca08df4fb..0ef61885b 100644 --- a/test/eca/llm_api_test.clj +++ b/test/eca/llm_api_test.clj @@ -704,8 +704,32 @@ (is (= 8000 (:delay-ms event))) (is (number? (:resets-at event))))))) -(deftest sync-rate-limit-waits-until-reset-without-cap-test - (testing "without rateLimitMaxWaitSeconds config, waits the full header reset even when long" +(deftest sync-rate-limit-default-max-wait-test + (testing "default rateLimitMaxWaitSeconds rejects a long Codex usage-limit reset" + (let [attempt* (atom 0) + error* (atom nil) + slept* (atom [])] + (with-redefs [eca.llm-api/prompt! (fn [_] + (swap! attempt* inc) + (let [reset-epoch-s (+ (quot (System/currentTimeMillis) 1000) 7200)] + {:error {:status 429 + :body {:error {:type "usage_limit_reached" + :resets_at reset-epoch-s}} + :message "OpenAI response status: 429"}})) + eca.llm-api/sleep-with-cancel (fn [delay-ms _] + (swap! slept* conj delay-ms) + true)] + (llm-api/sync-or-async-prompt! + (make-prompt-opts + {:stream false + :on-error (fn [error] (reset! error* error)) + :on-message-received identity}))) + (is (= 1 @attempt*)) + (is (empty? @slept*)) + (is (number? (:rate-limit-resets-at @error*)))))) + +(deftest rate-limit-default-max-wait-buffer-boundary-test + (testing "59-second provider reset is allowed because the buffered delay is exactly 60 seconds" (let [attempt* (atom 0) slept* (atom [])] (with-redefs [eca.llm-api/prompt! (fn [_] @@ -714,7 +738,7 @@ {:error {:status 429 :body "Rate limit exceeded" :message "LLM response status: 429" - :headers {"retry-after" "7200"}}} + :headers {"retry-after" "59"}}} {:output-text "success" :usage {:input-tokens 1 :output-tokens 1}}))) eca.llm-api/sleep-with-cancel (fn [delay-ms cancelled?] @@ -726,7 +750,29 @@ :on-error identity :on-message-received identity}))) (is (= 2 @attempt*)) - (is (= [7201000] @slept*) "2h from retry-after + 1s buffer, not capped")))) + (is (= [60000] @slept*)))) + + (testing "60-second provider reset is rejected because the safety buffer makes 61 seconds" + (let [attempt* (atom 0) + error* (atom nil) + slept* (atom [])] + (with-redefs [eca.llm-api/prompt! (fn [_] + (swap! attempt* inc) + {:error {:status 429 + :body "Rate limit exceeded" + :message "LLM response status: 429" + :headers {"retry-after" "60"}}}) + eca.llm-api/sleep-with-cancel (fn [delay-ms _] + (swap! slept* conj delay-ms) + true)] + (llm-api/sync-or-async-prompt! + (make-prompt-opts + {:stream false + :on-error (fn [error] (reset! error* error)) + :on-message-received identity}))) + (is (= 1 @attempt*)) + (is (empty? @slept*)) + (is (number? (:rate-limit-resets-at @error*)))))) (deftest rate-limit-max-wait-config-override-test (testing "provider rateLimitMaxWaitSeconds lower than reset wait disables the retry, exposing resets-at" diff --git a/test/eca/llm_providers/errors_test.clj b/test/eca/llm_providers/errors_test.clj index 88765ac9b..8424f36b8 100644 --- a/test/eca/llm_providers/errors_test.clj +++ b/test/eca/llm_providers/errors_test.clj @@ -399,33 +399,77 @@ (llm-providers.errors/rate-limit-wait {"x-ratelimit-reset" (str (+ now-ms 5000))} now-ms)))) - (testing "chatgpt codex exhausted window uses reset-at epoch" - (let [reset-epoch-s (+ (quot now-ms 1000) 1800)] - (is (= {:delay-ms 1800000 :resets-at (* reset-epoch-s 1000)} + (testing "chatgpt codex usage-limit reset accepts raw and decoded response bodies" + (let [reset-epoch-s (+ (quot now-ms 1000) 1800) + expected {:delay-ms 1800000 :resets-at (* reset-epoch-s 1000)} + headers {"x-codex-primary-used-percent" "100" + "x-codex-primary-reset-at" (str (+ reset-epoch-s 3600))}] + (doseq [[label body] + [["raw JSON string" + (str "{\"error\":{\"type\":\"usage_limit_reached\",\"resets_at\":" reset-epoch-s "}}")] + ["keyword-keyed decoded map" + {:error {:type "usage_limit_reached" :resets_at reset-epoch-s}}] + ["string-keyed decoded map" + {"error" {"type" "usage_limit_reached" "resets_at" reset-epoch-s}}]]] + (is (= expected + (llm-providers.errors/rate-limit-wait headers body now-ms)) + label)))) + + (testing "future retry-after takes precedence over chatgpt codex body reset" + (let [reset-epoch-s (+ (quot now-ms 1000) 1800) + body {:error {:type "usage_limit_reached" :resets_at reset-epoch-s}}] + (is (= {:delay-ms 7000 :resets-at (+ now-ms 7000)} (llm-providers.errors/rate-limit-wait - {"x-codex-primary-used-percent" "100" - "x-codex-primary-reset-at" (str reset-epoch-s) - "x-codex-secondary-used-percent" "0"} now-ms))))) - - (testing "chatgpt codex exhausted window falls back to reset-after-seconds" - (is (= {:delay-ms 3600000 :resets-at (+ now-ms 3600000)} - (llm-providers.errors/rate-limit-wait - {"x-codex-primary-used-percent" "100" - "x-codex-primary-reset-at" "" - "x-codex-primary-reset-after-seconds" "3600"} now-ms)))) + {"retry-after" "7"} + body + now-ms))))) + + (testing "expired retry-after falls through to a future chatgpt codex body reset" + (let [reset-epoch-s (+ (quot now-ms 1000) 1800) + body {:error {:type "usage_limit_reached" :resets_at reset-epoch-s}} + expired-http-date (.format java.time.format.DateTimeFormatter/RFC_1123_DATE_TIME + (.atZone (java.time.Instant/ofEpochMilli (- now-ms 60000)) + (java.time.ZoneId/of "GMT")))] + (doseq [retry-after ["0" expired-http-date]] + (is (= {:delay-ms 1800000 :resets-at (* reset-epoch-s 1000)} + (llm-providers.errors/rate-limit-wait + {"retry-after" retry-after} + body + now-ms)))))) + + (testing "past chatgpt codex body reset falls through to a future lower-priority reset" + (let [past-reset-epoch-s (- (quot now-ms 1000) 60) + future-reset-epoch-s (+ (quot now-ms 1000) 30)] + (is (= {:delay-ms 30000 :resets-at (* future-reset-epoch-s 1000)} + (llm-providers.errors/rate-limit-wait + {"anthropic-ratelimit-unified-reset" (str future-reset-epoch-s)} + {:error {:type "usage_limit_reached" :resets_at past-reset-epoch-s}} + now-ms))))) - (testing "chatgpt codex windows with headroom are not a wait signal" + (testing "chatgpt codex quota-window headers are not retry instructions" (is (nil? (llm-providers.errors/rate-limit-wait - {"x-codex-primary-used-percent" "0" - "x-codex-primary-reset-at" (str (+ (quot now-ms 1000) 604800)) - "x-codex-primary-reset-after-seconds" "604800"} now-ms)))) + {"x-codex-primary-used-percent" "100" + "x-codex-primary-reset-at" (str (+ (quot now-ms 1000) 3600)) + "x-codex-secondary-used-percent" "100" + "x-codex-secondary-reset-at" (str (+ (quot now-ms 1000) 604800))} + now-ms)))) - (testing "past resets, malformed or absent headers return nil" + (testing "past resets, malformed or absent reset data return nil" (is (nil? (llm-providers.errors/rate-limit-wait {"retry-after" "0"} now-ms))) (is (nil? (llm-providers.errors/rate-limit-wait {"retry-after" "garbage"} now-ms))) (is (nil? (llm-providers.errors/rate-limit-wait {"anthropic-ratelimit-requests-reset" "not-a-date"} now-ms))) (is (nil? (llm-providers.errors/rate-limit-wait {"anthropic-ratelimit-requests-reset" (instant-str (- now-ms 30000))} now-ms))) + (is (nil? (llm-providers.errors/rate-limit-wait + nil + "{\"error\":{\"type\":\"rate_limit_exceeded\",\"resets_at\":1000000060}}" + now-ms))) + (is (nil? (llm-providers.errors/rate-limit-wait + nil + {:error {:type "usage_limit_reached" + :resets_at (- (quot now-ms 1000) 60)}} + now-ms))) + (is (nil? (llm-providers.errors/rate-limit-wait nil "not-json" now-ms))) (is (nil? (llm-providers.errors/rate-limit-wait {} now-ms))) (is (nil? (llm-providers.errors/rate-limit-wait nil now-ms)))))) diff --git a/test/eca/llm_providers/openai_test.clj b/test/eca/llm_providers/openai_test.clj index fded20800..a08033037 100644 --- a/test/eca/llm_providers/openai_test.clj +++ b/test/eca/llm_providers/openai_test.clj @@ -36,6 +36,27 @@ (is (= {:output-text "Hello from responses!"} (select-keys response [:output-text])))))))) +(deftest base-responses-req-preserves-decoded-error-body-test + (testing "non-streaming 429 keeps the decoded JSON body in structured error data" + (let [error-body {:error {:type "usage_limit_reached" + :resets_at 2000000000}} + request-opts* (atom nil)] + (with-redefs [http/post (fn [_url opts] + (reset! request-opts* opts) + {:status 429 + :headers {"retry-after" "7"} + :body error-body})] + (let [result (#'llm-providers.openai/base-responses-request! + {:rid "r1" + :api-key "fake-key" + :api-url "http://localhost:1" + :body {:model "mymodel" :input "hi" :stream false} + :url-relative-path "/v1/responses"})] + (is (= :json (:as @request-opts*))) + (is (= 429 (get-in result [:error :status]))) + (is (= error-body (get-in result [:error :body]))) + (is (= "7" (get-in result [:error :headers "retry-after"])))))))) + (deftest oauth-authorize-test (testing "that OAuth token exchange is routed through the http proxy" (let [req* (atom nil)