From 476f51fb5415d106be9cce32fd32fafeceecf0d9 Mon Sep 17 00:00:00 2001 From: jroth1111 <250354952+jroth1111@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:25:19 +1000 Subject: [PATCH 1/4] fix(codex): preserve upstream error headers --- .../executor/codex_executor_execute.go | 4 +- .../executor/codex_executor_retry_test.go | 52 +++++++++++++++++ .../runtime/executor/codex_executor_stream.go | 2 +- .../codex_executor_stream_output_test.go | 56 +++++++++++++++++++ .../executor/codex_executor_terminal.go | 35 ++++++++++++ .../runtime/executor/codex_openai_images.go | 8 +-- 6 files changed, 150 insertions(+), 7 deletions(-) diff --git a/internal/runtime/executor/codex_executor_execute.go b/internal/runtime/executor/codex_executor_execute.go index 87279814d8e..4fb6f38e978 100644 --- a/internal/runtime/executor/codex_executor_execute.go +++ b/internal/runtime/executor/codex_executor_execute.go @@ -120,7 +120,7 @@ func (e *CodexExecutor) Execute(ctx context.Context, auth *cliproxyauth.Auth, re } helps.AppendAPIResponseChunk(ctx, e.cfg, b) helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), b)) - err = newCodexStatusErr(httpResp.StatusCode, b) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, b, httpResp.Header) return resp, err } data, errRead := io.ReadAll(httpResp.Body) @@ -274,7 +274,7 @@ func (e *CodexExecutor) executeCompact(ctx context.Context, auth *cliproxyauth.A b = applyCodexIdentityConfuseResponsePayload(b, identityState) helps.AppendAPIResponseChunk(ctx, e.cfg, b) helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), b)) - err = newCodexStatusErr(httpResp.StatusCode, b) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, b, httpResp.Header) return resp, err } data, err := io.ReadAll(httpResp.Body) diff --git a/internal/runtime/executor/codex_executor_retry_test.go b/internal/runtime/executor/codex_executor_retry_test.go index 2162b7bb369..2b497df7f22 100644 --- a/internal/runtime/executor/codex_executor_retry_test.go +++ b/internal/runtime/executor/codex_executor_retry_test.go @@ -61,6 +61,58 @@ func TestParseCodexRetryAfter(t *testing.T) { }) } +func TestNewCodexStatusErrWithHeadersPreservesRetryMetadata(t *testing.T) { + t.Run("delta seconds", func(t *testing.T) { + headers := http.Header{ + "Retry-After": {"7"}, + "X-Request-Id": {"req-test"}, + } + err := newCodexStatusErrWithHeaders( + http.StatusServiceUnavailable, + []byte(`{"error":{"type":"server_error","message":"overloaded"}}`), + headers, + ) + + if retryAfter := err.RetryAfter(); retryAfter == nil || *retryAfter != 7*time.Second { + t.Fatalf("RetryAfter() = %v, want 7s", retryAfter) + } + if got := err.Headers().Get("X-Request-Id"); got != "req-test" { + t.Fatalf("Headers().Get(X-Request-Id) = %q, want req-test", got) + } + + headers.Set("X-Request-Id", "mutated") + if got := err.Headers().Get("X-Request-Id"); got != "req-test" { + t.Fatalf("stored headers changed with source mutation: %q", got) + } + }) + + t.Run("http date", func(t *testing.T) { + retryAt := time.Now().Add(10 * time.Second).UTC().Truncate(time.Second) + err := newCodexStatusErrWithHeaders( + http.StatusServiceUnavailable, + []byte(`{"error":{"type":"server_error"}}`), + http.Header{"Retry-After": {retryAt.Format(http.TimeFormat)}}, + ) + + retryAfter := err.RetryAfter() + if retryAfter == nil || *retryAfter < 8*time.Second || *retryAfter > 10*time.Second { + t.Fatalf("RetryAfter() = %v, want approximately 10s", retryAfter) + } + }) + + t.Run("body retry metadata wins", func(t *testing.T) { + err := newCodexStatusErrWithHeaders( + http.StatusTooManyRequests, + []byte(`{"error":{"type":"usage_limit_reached","resets_in_seconds":120}}`), + http.Header{"Retry-After": {"7"}}, + ) + + if retryAfter := err.RetryAfter(); retryAfter == nil || *retryAfter != 120*time.Second { + t.Fatalf("RetryAfter() = %v, want body-derived 2m", retryAfter) + } + }) +} + func TestNewCodexStatusErrTreatsCapacityAsRetryableRateLimit(t *testing.T) { body := []byte(`{"error":{"message":"Selected model is at capacity. Please try a different model."}}`) diff --git a/internal/runtime/executor/codex_executor_stream.go b/internal/runtime/executor/codex_executor_stream.go index 8d5c8993096..10e734193b4 100644 --- a/internal/runtime/executor/codex_executor_stream.go +++ b/internal/runtime/executor/codex_executor_stream.go @@ -123,7 +123,7 @@ func (e *CodexExecutor) ExecuteStream(ctx context.Context, auth *cliproxyauth.Au } helps.AppendAPIResponseChunk(ctx, e.cfg, data) helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), data)) - err = newCodexStatusErr(httpResp.StatusCode, data) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, data, httpResp.Header) return nil, err } out := make(chan cliproxyexecutor.StreamChunk) diff --git a/internal/runtime/executor/codex_executor_stream_output_test.go b/internal/runtime/executor/codex_executor_stream_output_test.go index 477a9800a13..1b46e62ea93 100644 --- a/internal/runtime/executor/codex_executor_stream_output_test.go +++ b/internal/runtime/executor/codex_executor_stream_output_test.go @@ -18,6 +18,62 @@ import ( "github.com/tidwall/gjson" ) +func TestCodexExecutorHTTPErrorPreservesUpstreamHeaders(t *testing.T) { + for _, stream := range []bool{false, true} { + t.Run(map[bool]string{false: "execute", true: "execute_stream"}[stream], func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Retry-After", "7") + w.Header().Set("X-Request-Id", "req-test") + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"error":{"type":"server_error","message":"overloaded"}}`)) + })) + defer server.Close() + + executor := NewCodexExecutor(&config.Config{}) + auth := &cliproxyauth.Auth{Attributes: map[string]string{ + "base_url": server.URL, + "api_key": "test", + }} + req := cliproxyexecutor.Request{ + Model: "gpt-test", + Payload: []byte(`{"model":"gpt-test","input":"hello"}`), + } + opts := cliproxyexecutor.Options{ + SourceFormat: sdktranslator.FromString("openai-response"), + Stream: stream, + } + + var err error + if stream { + _, err = executor.ExecuteStream(context.Background(), auth, req, opts) + } else { + _, err = executor.Execute(context.Background(), auth, req, opts) + } + if err == nil { + t.Fatal("expected upstream 503 error") + } + if got := statusCodeFromTestError(t, err); got != http.StatusServiceUnavailable { + t.Fatalf("status code = %d, want %d", got, http.StatusServiceUnavailable) + } + retryErr, ok := err.(interface{ RetryAfter() *time.Duration }) + if !ok { + t.Fatalf("error %T does not expose RetryAfter()", err) + } + if retryAfter := retryErr.RetryAfter(); retryAfter == nil || *retryAfter != 7*time.Second { + t.Fatalf("RetryAfter() = %v, want 7s", retryAfter) + } + headerErr, ok := err.(interface{ Headers() http.Header }) + if !ok { + t.Fatalf("error %T does not expose Headers()", err) + } + if got := headerErr.Headers().Get("X-Request-Id"); got != "req-test" { + t.Fatalf("X-Request-Id = %q, want req-test", got) + } + }) + } +} + func TestCodexExecutorExecute_EmptyStreamCompletionOutputUsesOutputItemDone(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/event-stream") diff --git a/internal/runtime/executor/codex_executor_terminal.go b/internal/runtime/executor/codex_executor_terminal.go index 3ebc3d4ec9d..f12a2c167e2 100644 --- a/internal/runtime/executor/codex_executor_terminal.go +++ b/internal/runtime/executor/codex_executor_terminal.go @@ -4,6 +4,7 @@ import ( "bytes" "net/http" "sort" + "strconv" "strings" "time" @@ -261,6 +262,40 @@ func newCodexStatusErr(statusCode int, body []byte) statusErr { return err } +func newCodexStatusErrWithHeaders(statusCode int, body []byte, headers http.Header) statusErrWithHeaders { + err := newCodexStatusErr(statusCode, body) + if err.retryAfter == nil { + err.retryAfter = parseRetryAfterHeader(headers.Get("Retry-After"), time.Now()) + } + return statusErrWithHeaders{ + statusErr: err, + headers: headers.Clone(), + } +} + +func parseRetryAfterHeader(value string, now time.Time) *time.Duration { + value = strings.TrimSpace(value) + if value == "" { + return nil + } + if seconds, err := strconv.ParseInt(value, 10, 64); err == nil { + if seconds < 0 { + return nil + } + delay := time.Duration(seconds) * time.Second + return &delay + } + retryAt, err := http.ParseTime(value) + if err != nil { + return nil + } + delay := retryAt.Sub(now) + if delay < 0 { + delay = 0 + } + return &delay +} + func classifyCodexStatusError(statusCode int, body []byte) []byte { code, errType, ok := codexStatusErrorClassification(statusCode, body) if !ok { diff --git a/internal/runtime/executor/codex_openai_images.go b/internal/runtime/executor/codex_openai_images.go index 6a514a6a843..c48c4d765e0 100644 --- a/internal/runtime/executor/codex_openai_images.go +++ b/internal/runtime/executor/codex_openai_images.go @@ -140,7 +140,7 @@ func (e *CodexExecutor) executeOpenAIImage(ctx context.Context, auth *cliproxyau helps.AppendAPIResponseChunk(ctx, e.cfg, data) if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 { helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), data)) - err = newCodexStatusErr(httpResp.StatusCode, data) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, data, httpResp.Header) return resp, err } @@ -234,7 +234,7 @@ func (e *CodexExecutor) executeOpenAIImageStream(ctx context.Context, auth *clip data = applyCodexIdentityConfuseResponsePayload(data, identityState) helps.AppendAPIResponseChunk(ctx, e.cfg, data) helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), data)) - err = newCodexStatusErr(httpResp.StatusCode, data) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, data, httpResp.Header) return nil, err } @@ -367,7 +367,7 @@ func (e *CodexExecutor) executeDirectOpenAIImage(ctx context.Context, auth *clip helps.AppendAPIResponseChunk(ctx, e.cfg, data) if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 { helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), data)) - err = newCodexStatusErr(httpResp.StatusCode, data) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, data, httpResp.Header) return resp, err } @@ -425,7 +425,7 @@ func (e *CodexExecutor) executeDirectOpenAIImageStream(ctx context.Context, auth data = applyCodexIdentityConfuseResponsePayload(data, identityState) helps.AppendAPIResponseChunk(ctx, e.cfg, data) helps.LogWithRequestID(ctx).Debugf("request error, error status: %d, error message: %s", httpResp.StatusCode, helps.SummarizeErrorBody(httpResp.Header.Get("Content-Type"), data)) - err = newCodexStatusErr(httpResp.StatusCode, data) + err = newCodexStatusErrWithHeaders(httpResp.StatusCode, data, httpResp.Header) return nil, err } From d530e39a482ce57128b477ab0efcc14316eb34a6 Mon Sep 17 00:00:00 2001 From: jroth1111 <250354952+jroth1111@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:28:51 +1000 Subject: [PATCH 2/4] fix(codex): honor error header safety and retry hints Co-Authored-By: Claude --- sdk/api/handlers/handlers_execution.go | 4 +- .../handlers_execution_error_headers_test.go | 47 +++++++++++++++ sdk/cliproxy/auth/conductor_cooldown.go | 14 ++++- sdk/cliproxy/auth/conductor_overrides_test.go | 57 +++++++++++++++++++ 4 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 sdk/api/handlers/handlers_execution_error_headers_test.go diff --git a/sdk/api/handlers/handlers_execution.go b/sdk/api/handlers/handlers_execution.go index 7c25ab14612..68b51c7cdae 100644 --- a/sdk/api/handlers/handlers_execution.go +++ b/sdk/api/handlers/handlers_execution.go @@ -310,9 +310,7 @@ func executionErrorMessage(err error) *interfaces.ErrorMessage { } var addon http.Header if he, ok := err.(interface{ Headers() http.Header }); ok && he != nil { - if hdr := he.Headers(); hdr != nil { - addon = hdr.Clone() - } + addon = FilterUpstreamHeaders(he.Headers()) } return &interfaces.ErrorMessage{StatusCode: status, Error: err, Addon: addon} } diff --git a/sdk/api/handlers/handlers_execution_error_headers_test.go b/sdk/api/handlers/handlers_execution_error_headers_test.go new file mode 100644 index 00000000000..0d64a7ed6b5 --- /dev/null +++ b/sdk/api/handlers/handlers_execution_error_headers_test.go @@ -0,0 +1,47 @@ +package handlers + +import ( + "net/http" + "testing" +) + +type executionHeaderError struct { + headers http.Header +} + +func (e executionHeaderError) Error() string { return "upstream unavailable" } +func (e executionHeaderError) StatusCode() int { return http.StatusServiceUnavailable } +func (e executionHeaderError) Headers() http.Header { + return e.headers.Clone() +} + +func TestExecutionErrorMessageFiltersUpstreamHeaders(t *testing.T) { + err := executionHeaderError{headers: http.Header{ + "Retry-After": {"120"}, + "X-Request-Id": {"request-123"}, + "Content-Length": {"999"}, + "Set-Cookie": {"session=secret"}, + "Connection": {"X-Connection-Only"}, + "X-Connection-Only": {"secret"}, + "X-Litellm-Trace": {"gateway"}, + }} + + msg := executionErrorMessage(err) + if msg == nil || msg.Error == nil || msg.Error.Error() != err.Error() { + t.Fatalf("error message = %#v, want original error", msg) + } + if msg.StatusCode != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want %d", msg.StatusCode, http.StatusServiceUnavailable) + } + if got := msg.Addon.Get("Retry-After"); got != "120" { + t.Fatalf("Retry-After = %q, want 120", got) + } + if got := msg.Addon.Get("X-Request-Id"); got != "request-123" { + t.Fatalf("X-Request-Id = %q, want request-123", got) + } + for _, key := range []string{"Content-Length", "Set-Cookie", "Connection", "X-Connection-Only", "X-Litellm-Trace"} { + if values := msg.Addon.Values(key); len(values) != 0 { + t.Fatalf("%s leaked through filtered error headers: %v", key, values) + } + } +} diff --git a/sdk/cliproxy/auth/conductor_cooldown.go b/sdk/cliproxy/auth/conductor_cooldown.go index 7e143ed30f2..a45335ad4c6 100644 --- a/sdk/cliproxy/auth/conductor_cooldown.go +++ b/sdk/cliproxy/auth/conductor_cooldown.go @@ -93,6 +93,16 @@ func recoverableFailureRetryAfter(now time.Time, disableCooling bool) time.Time return nextTransientErrorRetryAfter(now) } +func transientFailureRetryAfter(now time.Time, retryAfter *time.Duration, disableCooling bool) time.Time { + if disableCooling { + return time.Time{} + } + if retryAfter != nil && *retryAfter >= 0 { + return now.Add(*retryAfter) + } + return nextTransientErrorRetryAfter(now) +} + // SetConfig updates the runtime config snapshot used by request-time helpers. // Callers should provide the latest config on reload so per-credential alias mapping stays in sync. func (m *Manager) SetConfig(cfg *internalconfig.Config) { @@ -827,7 +837,7 @@ func (m *Manager) MarkResult(ctx context.Context, result Result) { setModelQuota = true } case 408, 500, 502, 503, 504: - state.NextRetryAfter = recoverableFailureRetryAfter(now, disableCooling) + state.NextRetryAfter = transientFailureRetryAfter(now, result.RetryAfter, disableCooling) state.Unavailable = !state.NextRetryAfter.IsZero() default: state.NextRetryAfter = recoverableFailureRetryAfter(now, disableCooling) @@ -1655,7 +1665,7 @@ func applyAuthFailureState(auth *Auth, resultErr *Error, retryAfter *time.Durati auth.NextRetryAfter = next case 408, 500, 502, 503, 504: auth.StatusMessage = "transient upstream error" - auth.NextRetryAfter = recoverableFailureRetryAfter(now, disableCooling) + auth.NextRetryAfter = transientFailureRetryAfter(now, retryAfter, disableCooling) auth.Unavailable = !auth.NextRetryAfter.IsZero() default: if auth.StatusMessage == "" { diff --git a/sdk/cliproxy/auth/conductor_overrides_test.go b/sdk/cliproxy/auth/conductor_overrides_test.go index 110506b6ea7..b2f92743cba 100644 --- a/sdk/cliproxy/auth/conductor_overrides_test.go +++ b/sdk/cliproxy/auth/conductor_overrides_test.go @@ -788,6 +788,63 @@ func TestManager_MarkResult_TransientErrorCooldownDefault(t *testing.T) { } } +func TestManager_MarkResult_TransientErrorCooldownUsesRetryAfter(t *testing.T) { + prevQuota := quotaCooldownDisabled.Load() + quotaCooldownDisabled.Store(false) + prevTransient := transientErrorCooldownSeconds.Load() + SetTransientErrorCooldownSeconds(0) + t.Cleanup(func() { + quotaCooldownDisabled.Store(prevQuota) + transientErrorCooldownSeconds.Store(prevTransient) + }) + + retryAfter := 2 * time.Minute + m := NewManager(nil, nil, nil) + modelAuth := &Auth{ID: "auth-transient-retry-after-model", Provider: "codex"} + if _, errRegister := m.Register(context.Background(), modelAuth); errRegister != nil { + t.Fatalf("register model auth: %v", errRegister) + } + model := "gpt-retry-after" + m.MarkResult(context.Background(), Result{ + AuthID: modelAuth.ID, + Provider: modelAuth.Provider, + Model: model, + Success: false, + Error: &Error{HTTPStatus: http.StatusServiceUnavailable, Message: "unavailable"}, + RetryAfter: &retryAfter, + }) + + updatedModel, okModel := m.GetByID(modelAuth.ID) + if !okModel || updatedModel == nil || updatedModel.ModelStates[model] == nil { + t.Fatal("expected model cooldown state") + } + modelWait := time.Until(updatedModel.ModelStates[model].NextRetryAfter) + if modelWait < 115*time.Second || modelWait > 125*time.Second { + t.Fatalf("model Retry-After cooldown = %v, want ~2m", modelWait) + } + + authLevel := &Auth{ID: "auth-transient-retry-after-auth", Provider: "codex"} + if _, errRegister := m.Register(context.Background(), authLevel); errRegister != nil { + t.Fatalf("register auth-level auth: %v", errRegister) + } + m.MarkResult(context.Background(), Result{ + AuthID: authLevel.ID, + Provider: authLevel.Provider, + Success: false, + Error: &Error{HTTPStatus: http.StatusServiceUnavailable, Message: "unavailable"}, + RetryAfter: &retryAfter, + }) + + updatedAuth, okAuth := m.GetByID(authLevel.ID) + if !okAuth || updatedAuth == nil { + t.Fatal("expected auth cooldown state") + } + authWait := time.Until(updatedAuth.NextRetryAfter) + if authWait < 115*time.Second || authWait > 125*time.Second { + t.Fatalf("auth Retry-After cooldown = %v, want ~2m", authWait) + } +} + func TestManager_MarkResult_TransientErrorCooldownDisabled(t *testing.T) { prevQuota := quotaCooldownDisabled.Load() quotaCooldownDisabled.Store(false) From 0dc872490242c81ead302c52a53b93463de83044 Mon Sep 17 00:00:00 2001 From: jroth1111 <250354952+jroth1111@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:40:38 +1000 Subject: [PATCH 3/4] test(codex): cover error header passthrough modes Co-Authored-By: Claude --- .../handlers_execution_error_headers_test.go | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/sdk/api/handlers/handlers_execution_error_headers_test.go b/sdk/api/handlers/handlers_execution_error_headers_test.go index 0d64a7ed6b5..6f899cf2a03 100644 --- a/sdk/api/handlers/handlers_execution_error_headers_test.go +++ b/sdk/api/handlers/handlers_execution_error_headers_test.go @@ -2,7 +2,11 @@ package handlers import ( "net/http" + "net/http/httptest" "testing" + + "github.com/gin-gonic/gin" + sdkconfig "github.com/router-for-me/CLIProxyAPI/v7/sdk/config" ) type executionHeaderError struct { @@ -15,8 +19,8 @@ func (e executionHeaderError) Headers() http.Header { return e.headers.Clone() } -func TestExecutionErrorMessageFiltersUpstreamHeaders(t *testing.T) { - err := executionHeaderError{headers: http.Header{ +func newExecutionHeaderError() executionHeaderError { + return executionHeaderError{headers: http.Header{ "Retry-After": {"120"}, "X-Request-Id": {"request-123"}, "Content-Length": {"999"}, @@ -25,6 +29,10 @@ func TestExecutionErrorMessageFiltersUpstreamHeaders(t *testing.T) { "X-Connection-Only": {"secret"}, "X-Litellm-Trace": {"gateway"}, }} +} + +func TestExecutionErrorMessageFiltersUpstreamHeaders(t *testing.T) { + err := newExecutionHeaderError() msg := executionErrorMessage(err) if msg == nil || msg.Error == nil || msg.Error.Error() != err.Error() { @@ -45,3 +53,39 @@ func TestExecutionErrorMessageFiltersUpstreamHeaders(t *testing.T) { } } } + +func TestExecutionErrorHeadersRespectPassthroughToggle(t *testing.T) { + for _, enabled := range []bool{false, true} { + t.Run(map[bool]string{false: "disabled", true: "enabled"}[enabled], func(t *testing.T) { + gin.SetMode(gin.TestMode) + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest(http.MethodPost, "/v1/messages", nil) + + handler := NewBaseAPIHandlers(&sdkconfig.SDKConfig{PassthroughHeaders: enabled}, nil) + handler.WriteErrorResponse(ctx, executionErrorMessage(newExecutionHeaderError())) + + if recorder.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want %d", recorder.Code, http.StatusServiceUnavailable) + } + wantRetryAfter := "" + wantRequestID := "" + if enabled { + wantRetryAfter = "120" + wantRequestID = "request-123" + } + if got := recorder.Header().Get("Retry-After"); got != wantRetryAfter { + t.Fatalf("Retry-After = %q, want %q", got, wantRetryAfter) + } + if got := recorder.Header().Get("X-Request-Id"); got != wantRequestID { + t.Fatalf("X-Request-Id = %q, want %q", got, wantRequestID) + } + if got := recorder.Header().Get("Set-Cookie"); got != "" { + t.Fatalf("Set-Cookie leaked: %q", got) + } + if got := recorder.Header().Get("Content-Length"); got == "999" { + t.Fatalf("upstream Content-Length leaked: %q", got) + } + }) + } +} From e50432d518468344b42f90775bee64832631c02e Mon Sep 17 00:00:00 2001 From: jroth1111 <250354952+jroth1111@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:10:48 +1000 Subject: [PATCH 4/4] fix(codex): close error metadata edge cases --- .../executor/codex_executor_retry_test.go | 12 ++++++++ .../executor/codex_executor_terminal.go | 4 +++ .../openai/openai_responses_websocket.go | 2 +- .../openai_responses_websocket_forward.go | 16 +++++++--- .../openai/openai_responses_websocket_test.go | 30 +++++++++++++++++++ sdk/cliproxy/auth/conductor_cooldown.go | 2 +- sdk/cliproxy/auth/conductor_overrides_test.go | 21 +++++++------ 7 files changed, 72 insertions(+), 15 deletions(-) diff --git a/internal/runtime/executor/codex_executor_retry_test.go b/internal/runtime/executor/codex_executor_retry_test.go index 2b497df7f22..c335e8e5a9f 100644 --- a/internal/runtime/executor/codex_executor_retry_test.go +++ b/internal/runtime/executor/codex_executor_retry_test.go @@ -100,6 +100,18 @@ func TestNewCodexStatusErrWithHeadersPreservesRetryMetadata(t *testing.T) { } }) + t.Run("overflowing delta seconds", func(t *testing.T) { + err := newCodexStatusErrWithHeaders( + http.StatusServiceUnavailable, + []byte(`{"error":{"type":"server_error"}}`), + http.Header{"Retry-After": {"9999999999"}}, + ) + + if retryAfter := err.RetryAfter(); retryAfter != nil { + t.Fatalf("RetryAfter() = %v, want nil for overflowing delta seconds", *retryAfter) + } + }) + t.Run("body retry metadata wins", func(t *testing.T) { err := newCodexStatusErrWithHeaders( http.StatusTooManyRequests, diff --git a/internal/runtime/executor/codex_executor_terminal.go b/internal/runtime/executor/codex_executor_terminal.go index f12a2c167e2..18ca726110b 100644 --- a/internal/runtime/executor/codex_executor_terminal.go +++ b/internal/runtime/executor/codex_executor_terminal.go @@ -282,6 +282,10 @@ func parseRetryAfterHeader(value string, now time.Time) *time.Duration { if seconds < 0 { return nil } + const maxRetryAfterSeconds = int64(time.Duration(1<<63-1) / time.Second) + if seconds > maxRetryAfterSeconds { + return nil + } delay := time.Duration(seconds) * time.Second return &delay } diff --git a/sdk/api/handlers/openai/openai_responses_websocket.go b/sdk/api/handlers/openai/openai_responses_websocket.go index afcd7b8e6e9..516e2b8953a 100644 --- a/sdk/api/handlers/openai/openai_responses_websocket.go +++ b/sdk/api/handlers/openai/openai_responses_websocket.go @@ -436,7 +436,7 @@ func (h *OpenAIResponsesAPIHandler) ResponsesWebsocket(c *gin.Context) { if errMsg != nil { h.LoggingAPIResponseError(context.WithValue(context.Background(), "gin", c), errMsg) markAPIResponseTimestamp(c) - errorPayload, errWrite := writeResponsesWebsocketError(writer, wsTimelineLog, errMsg) + errorPayload, errWrite := writeResponsesWebsocketError(writer, wsTimelineLog, errMsg, handlers.PassthroughHeadersEnabled(h.Cfg)) log.Infof( "responses websocket: downstream_out id=%s type=%d event=%s payload=%s", passthroughSessionID, diff --git a/sdk/api/handlers/openai/openai_responses_websocket_forward.go b/sdk/api/handlers/openai/openai_responses_websocket_forward.go index b465201bcf2..fc563a5d4a8 100644 --- a/sdk/api/handlers/openai/openai_responses_websocket_forward.go +++ b/sdk/api/handlers/openai/openai_responses_websocket_forward.go @@ -74,7 +74,7 @@ func (h *OpenAIResponsesAPIHandler) forwardResponsesWebsocket( } return completedOutput, completedResponseID, sortedStringSet(pendingToolCallIDs), errMsg, websocket.ErrCloseSent } - errorPayload, errWrite := writeResponsesWebsocketError(writer, wsTimelineLog, errMsg) + errorPayload, errWrite := writeResponsesWebsocketError(writer, wsTimelineLog, errMsg, handlers.PassthroughHeadersEnabled(h.Cfg)) log.Infof( "responses websocket: downstream_out id=%s type=%d event=%s payload=%s", sessionID, @@ -108,7 +108,7 @@ func (h *OpenAIResponsesAPIHandler) forwardResponsesWebsocket( } h.LoggingAPIResponseError(context.WithValue(context.Background(), "gin", c), errMsg) markAPIResponseTimestamp(c) - errorPayload, errWrite := writeResponsesWebsocketError(writer, wsTimelineLog, errMsg) + errorPayload, errWrite := writeResponsesWebsocketError(writer, wsTimelineLog, errMsg, handlers.PassthroughHeadersEnabled(h.Cfg)) log.Infof( "responses websocket: downstream_out id=%s type=%d event=%s payload=%s", sessionID, @@ -478,7 +478,7 @@ func websocketJSONPayloadsFromChunk(chunk []byte) [][]byte { return payloads } -func writeResponsesWebsocketError(writer *responsesWebsocketWriter, wsTimelineLog websocketTimelineAppender, errMsg *interfaces.ErrorMessage) ([]byte, error) { +func buildResponsesWebsocketErrorPayload(errMsg *interfaces.ErrorMessage, passthroughHeaders bool) ([]byte, error) { status := http.StatusInternalServerError errText := http.StatusText(status) if errMsg != nil { @@ -503,7 +503,7 @@ func writeResponsesWebsocketError(writer *responsesWebsocketWriter, wsTimelineLo return nil, errSet } - if errMsg != nil && errMsg.Addon != nil { + if passthroughHeaders && errMsg != nil && errMsg.Addon != nil { headers := []byte(`{}`) hasHeaders := false for key, values := range errMsg.Addon { @@ -548,5 +548,13 @@ func writeResponsesWebsocketError(writer *responsesWebsocketWriter, wsTimelineLo } } + return payload, nil +} + +func writeResponsesWebsocketError(writer *responsesWebsocketWriter, wsTimelineLog websocketTimelineAppender, errMsg *interfaces.ErrorMessage, passthroughHeaders bool) ([]byte, error) { + payload, err := buildResponsesWebsocketErrorPayload(errMsg, passthroughHeaders) + if err != nil { + return nil, err + } return payload, writeResponsesWebsocketPayload(writer, wsTimelineLog, payload, time.Now()) } diff --git a/sdk/api/handlers/openai/openai_responses_websocket_test.go b/sdk/api/handlers/openai/openai_responses_websocket_test.go index 581efd0be12..52431b84215 100644 --- a/sdk/api/handlers/openai/openai_responses_websocket_test.go +++ b/sdk/api/handlers/openai/openai_responses_websocket_test.go @@ -36,6 +36,36 @@ type homeResponsesWebsocketDispatcher struct { calls atomic.Int32 } +func TestBuildResponsesWebsocketErrorPayloadHonorsHeaderPassthrough(t *testing.T) { + errMsg := &interfaces.ErrorMessage{ + StatusCode: http.StatusServiceUnavailable, + Error: errors.New("upstream unavailable"), + Addon: http.Header{ + "Retry-After": {"3"}, + "X-Request-Id": {"req-test"}, + }, + } + + withoutHeaders, err := buildResponsesWebsocketErrorPayload(errMsg, false) + if err != nil { + t.Fatalf("build payload without passthrough: %v", err) + } + if gjson.GetBytes(withoutHeaders, "headers").Exists() { + t.Fatalf("headers should be omitted when passthrough is disabled: %s", withoutHeaders) + } + + withHeaders, err := buildResponsesWebsocketErrorPayload(errMsg, true) + if err != nil { + t.Fatalf("build payload with passthrough: %v", err) + } + if got := gjson.GetBytes(withHeaders, "headers.Retry-After").String(); got != "3" { + t.Fatalf("Retry-After = %q, want 3", got) + } + if got := gjson.GetBytes(withHeaders, "headers.X-Request-Id").String(); got != "req-test" { + t.Fatalf("X-Request-Id = %q, want req-test", got) + } +} + func (*homeResponsesWebsocketDispatcher) HeartbeatOK() bool { return true } func (d *homeResponsesWebsocketDispatcher) RPopAuth(context.Context, string, string, http.Header, int) ([]byte, error) { diff --git a/sdk/cliproxy/auth/conductor_cooldown.go b/sdk/cliproxy/auth/conductor_cooldown.go index a45335ad4c6..026727821f8 100644 --- a/sdk/cliproxy/auth/conductor_cooldown.go +++ b/sdk/cliproxy/auth/conductor_cooldown.go @@ -94,7 +94,7 @@ func recoverableFailureRetryAfter(now time.Time, disableCooling bool) time.Time } func transientFailureRetryAfter(now time.Time, retryAfter *time.Duration, disableCooling bool) time.Time { - if disableCooling { + if disableCooling || transientErrorCooldownSeconds.Load() < 0 { return time.Time{} } if retryAfter != nil && *retryAfter >= 0 { diff --git a/sdk/cliproxy/auth/conductor_overrides_test.go b/sdk/cliproxy/auth/conductor_overrides_test.go index b2f92743cba..347e421fff0 100644 --- a/sdk/cliproxy/auth/conductor_overrides_test.go +++ b/sdk/cliproxy/auth/conductor_overrides_test.go @@ -856,6 +856,7 @@ func TestManager_MarkResult_TransientErrorCooldownDisabled(t *testing.T) { }) m := NewManager(nil, nil, nil) + retryAfter := 2 * time.Minute modelAuth := &Auth{ ID: "auth-transient-model-disabled", @@ -867,11 +868,12 @@ func TestManager_MarkResult_TransientErrorCooldownDisabled(t *testing.T) { model := "test-model-transient-disabled" m.MarkResult(context.Background(), Result{ - AuthID: modelAuth.ID, - Provider: modelAuth.Provider, - Model: model, - Success: false, - Error: &Error{HTTPStatus: http.StatusBadGateway, Message: "bad gateway"}, + AuthID: modelAuth.ID, + Provider: modelAuth.Provider, + Model: model, + Success: false, + Error: &Error{HTTPStatus: http.StatusBadGateway, Message: "bad gateway"}, + RetryAfter: &retryAfter, }) updatedModelAuth, okModelAuth := m.GetByID(modelAuth.ID) @@ -895,10 +897,11 @@ func TestManager_MarkResult_TransientErrorCooldownDisabled(t *testing.T) { } m.MarkResult(context.Background(), Result{ - AuthID: authLevelAuth.ID, - Provider: authLevelAuth.Provider, - Success: false, - Error: &Error{HTTPStatus: http.StatusServiceUnavailable, Message: "unavailable"}, + AuthID: authLevelAuth.ID, + Provider: authLevelAuth.Provider, + Success: false, + Error: &Error{HTTPStatus: http.StatusServiceUnavailable, Message: "unavailable"}, + RetryAfter: &retryAfter, }) updatedAuthLevel, okAuthLevel := m.GetByID(authLevelAuth.ID)