mirror of
https://github.com/Wei-Shaw/sub2api.git
synced 2026-11-06 00:48:36 +00:00
fix(openai): mark OAuth accounts unschedulable on Codex models manifest 401
The Codex models manifest path returned upstream 401s straight to the client without feeding them into the account state machinery. A revoked or invalidated OAuth account therefore stayed active and schedulable, kept being selected for subsequent /models requests, and produced repeated 502s until an admin ran a manual connection test (#4544). - Route ChatGPT-backend manifest 401s through the shared upstream-error handling: token cache invalidation, temp-unschedulable cooldown for refreshable OAuth accounts, permanent disable for token_revoked/token_invalidated, plus the runtime scheduling block. - Treat ChatGPT-backend manifest 401s as failover-eligible so the current /models request can switch to a healthy account instead of returning 502. Custom API key upstream 401s keep the existing no-failover, no-disable behavior since their /models auth is not authoritative for the account. - Skip Agent Identity accounts: their 401s can be task-scoped and have a dedicated recovery flow. - Attach the selected account to the ops error-log context so /models failures record the account_id. Fixes #4544 Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
1 parent
d4b9797ff7
commit
3ed2873f14
3 files changed
+186
-2
No files matched your search
@@ -53,6 +53,8 @@ func (h *OpenAIGatewayHandler) CodexModels(c *gin.Context) {
|
|||||||
h.errorResponse(c, http.StatusServiceUnavailable, "upstream_error", "No available OpenAI accounts")
|
h.errorResponse(c, http.StatusServiceUnavailable, "upstream_error", "No available OpenAI accounts")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// 让 ops 错误日志携带实际选中的上游账号,便于定位失效账号(#4544)。
|
||||||
|
setOpsSelectedAccount(c, account.ID, account.Platform)
|
||||||
|
|
||||||
manifest, err := h.gatewayService.FetchCodexModelsManifest(c.Request.Context(), account, c.Query("client_version"), c.GetHeader("If-None-Match"))
|
manifest, err := h.gatewayService.FetchCodexModelsManifest(c.Request.Context(), account, c.Query("client_version"), c.GetHeader("If-None-Match"))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -47,6 +47,7 @@ type codexModelsManifestUpstreamError struct {
|
|||||||
err error
|
err error
|
||||||
retryable bool
|
retryable bool
|
||||||
statusCode int
|
statusCode int
|
||||||
|
headers http.Header
|
||||||
body []byte
|
body []byte
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -56,7 +57,13 @@ func (e *codexModelsManifestUpstreamError) Unwrap() error { return e.err }
|
|||||||
|
|
||||||
// IsRetryableCodexModelsManifestError reports whether another selected account
|
// IsRetryableCodexModelsManifestError reports whether another selected account
|
||||||
// may succeed without changing the request. Configuration and upstream 4xx
|
// may succeed without changing the request. Configuration and upstream 4xx
|
||||||
// responses, except 429, are intentionally not retried.
|
// responses, except 429 and ChatGPT-backend 401, are intentionally not
|
||||||
|
// retried. A manifest 401 from the ChatGPT Codex backend reflects the selected
|
||||||
|
// OAuth account's upstream token rather than the client request (the client's
|
||||||
|
// own API key was already validated locally), so a different account may still
|
||||||
|
// serve the manifest. Custom API key upstreams keep the old no-failover 401
|
||||||
|
// behavior because their /models auth semantics are not authoritative for the
|
||||||
|
// account.
|
||||||
func IsRetryableCodexModelsManifestError(err error) bool {
|
func IsRetryableCodexModelsManifestError(err error) bool {
|
||||||
var upstreamErr *codexModelsManifestUpstreamError
|
var upstreamErr *codexModelsManifestUpstreamError
|
||||||
return errors.As(err, &upstreamErr) && upstreamErr.retryable
|
return errors.As(err, &upstreamErr) && upstreamErr.retryable
|
||||||
@@ -322,6 +329,7 @@ func (s *OpenAIGatewayService) FetchCodexModelsManifest(ctx context.Context, acc
|
|||||||
}
|
}
|
||||||
manifest, fetchErr := s.fetchCodexModelsManifestUpstream(ctx, request, ifNoneMatch)
|
manifest, fetchErr := s.fetchCodexModelsManifestUpstream(ctx, request, ifNoneMatch)
|
||||||
if !credAccount.IsOpenAIAgentIdentity() || !isAgentIdentityTaskInvalidCodexModelsError(fetchErr) {
|
if !credAccount.IsOpenAIAgentIdentity() || !isAgentIdentityTaskInvalidCodexModelsError(fetchErr) {
|
||||||
|
s.handleCodexModelsManifestAccountAuthError(ctx, account, credAccount, fetchErr)
|
||||||
return manifest, fetchErr
|
return manifest, fetchErr
|
||||||
}
|
}
|
||||||
expectedTaskID := strings.TrimSpace(credAccount.GetCredential("task_id"))
|
expectedTaskID := strings.TrimSpace(credAccount.GetCredential("task_id"))
|
||||||
@@ -349,6 +357,37 @@ func isAgentIdentityTaskInvalidCodexModelsError(err error) bool {
|
|||||||
isAgentIdentityTaskInvalidHTTPResponse(upstreamErr.statusCode, upstreamErr.body)
|
isAgentIdentityTaskInvalidHTTPResponse(upstreamErr.statusCode, upstreamErr.body)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// handleCodexModelsManifestAccountAuthError feeds manifest 401s from the
|
||||||
|
// ChatGPT Codex backend into the shared upstream-error state machinery
|
||||||
|
// (token cache invalidation, temp-unschedulable cooldown, or permanent
|
||||||
|
// disable for token_revoked/token_invalidated). Without this, an account
|
||||||
|
// whose OAuth token was revoked upstream stays active and schedulable and
|
||||||
|
// keeps being selected for every subsequent /models request (#4544).
|
||||||
|
//
|
||||||
|
// Scope is deliberately limited to plain OAuth accounts: the manifest
|
||||||
|
// endpoint authenticates with the same token as /responses forwarding, so a
|
||||||
|
// 401 is authoritative for the account. Agent Identity accounts are excluded
|
||||||
|
// because their 401s can be task-scoped and have a dedicated recovery flow,
|
||||||
|
// and API key manifests come from custom upstreams whose /models auth may
|
||||||
|
// diverge from their chat endpoints.
|
||||||
|
func (s *OpenAIGatewayService) handleCodexModelsManifestAccountAuthError(ctx context.Context, account, credAccount *Account, err error) {
|
||||||
|
if s == nil || account == nil || err == nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if credAccount == nil || !credAccount.IsOpenAIOAuth() || credAccount.IsOpenAIAgentIdentity() {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
var upstreamErr *codexModelsManifestUpstreamError
|
||||||
|
if !errors.As(err, &upstreamErr) || upstreamErr.statusCode != http.StatusUnauthorized {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
headers := upstreamErr.headers
|
||||||
|
if headers == nil {
|
||||||
|
headers = http.Header{}
|
||||||
|
}
|
||||||
|
s.handleOpenAIAccountUpstreamError(ctx, account, upstreamErr.statusCode, headers, upstreamErr.body)
|
||||||
|
}
|
||||||
|
|
||||||
func (s *OpenAIGatewayService) fetchCachedAPIKeyCodexModelsManifest(ctx context.Context, request codexModelsManifestRequest, ifNoneMatch string) (*CodexModelsManifest, error) {
|
func (s *OpenAIGatewayService) fetchCachedAPIKeyCodexModelsManifest(ctx context.Context, request codexModelsManifestRequest, ifNoneMatch string) (*CodexModelsManifest, error) {
|
||||||
if err := ctx.Err(); err != nil {
|
if err := ctx.Err(); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
@@ -450,8 +489,10 @@ func (s *OpenAIGatewayService) fetchCodexModelsManifestUpstream(ctx context.Cont
|
|||||||
return nil, &codexModelsManifestUpstreamError{
|
return nil, &codexModelsManifestUpstreamError{
|
||||||
err: infraerrors.Newf(http.StatusBadGateway, "OPENAI_CODEX_MODELS_UPSTREAM_FAILED", "codex models manifest upstream error %d: %s", resp.StatusCode, message),
|
err: infraerrors.Newf(http.StatusBadGateway, "OPENAI_CODEX_MODELS_UPSTREAM_FAILED", "codex models manifest upstream error %d: %s", resp.StatusCode, message),
|
||||||
statusCode: resp.StatusCode,
|
statusCode: resp.StatusCode,
|
||||||
|
headers: resp.Header.Clone(),
|
||||||
body: body,
|
body: body,
|
||||||
retryable: resp.StatusCode == http.StatusTooManyRequests ||
|
retryable: (resp.StatusCode == http.StatusUnauthorized && !request.useAPIKeyUpstream) ||
|
||||||
|
resp.StatusCode == http.StatusTooManyRequests ||
|
||||||
(resp.StatusCode >= http.StatusInternalServerError && resp.StatusCode < 600),
|
(resp.StatusCode >= http.StatusInternalServerError && resp.StatusCode < 600),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1094,6 +1094,147 @@ func TestFetchCodexModelsManifestAPIKeyRejectsBaseURLFragment(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// codexModelsAccountStateRepo records account state transitions triggered by
|
||||||
|
// manifest upstream errors (#4544).
|
||||||
|
type codexModelsAccountStateRepo struct {
|
||||||
|
AccountRepository
|
||||||
|
mu sync.Mutex
|
||||||
|
setErrorCalls int
|
||||||
|
lastErrorMsg string
|
||||||
|
setTempUnschedCalls int
|
||||||
|
lastTempReason string
|
||||||
|
}
|
||||||
|
|
||||||
|
func (r *codexModelsAccountStateRepo) SetError(_ context.Context, _ int64, errorMsg string) error {
|
||||||
|
r.mu.Lock()
|
||||||
|
defer r.mu.Unlock()
|
||||||
|
r.setErrorCalls++
|
||||||
|
r.lastErrorMsg = errorMsg
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func (r *codexModelsAccountStateRepo) SetTempUnschedulable(_ context.Context, _ int64, _ time.Time, reason string) error {
|
||||||
|
r.mu.Lock()
|
||||||
|
defer r.mu.Unlock()
|
||||||
|
r.setTempUnschedCalls++
|
||||||
|
r.lastTempReason = reason
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func newCodexModels401TestService(repo AccountRepository) *OpenAIGatewayService {
|
||||||
|
rateLimitService := NewRateLimitService(repo, nil, &config.Config{}, nil, nil)
|
||||||
|
s := &OpenAIGatewayService{rateLimitService: rateLimitService}
|
||||||
|
rateLimitService.SetAccountRuntimeBlocker(s)
|
||||||
|
return s
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFetchCodexModelsManifestOAuth401MarksAccountUnschedulable(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusUnauthorized)
|
||||||
|
_, _ = w.Write([]byte(`{"detail":{"message":"invalid token"}}`))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
original := chatgptCodexModelsURL
|
||||||
|
chatgptCodexModelsURL = server.URL
|
||||||
|
defer func() { chatgptCodexModelsURL = original }()
|
||||||
|
|
||||||
|
repo := &codexModelsAccountStateRepo{}
|
||||||
|
s := newCodexModels401TestService(repo)
|
||||||
|
account := newCodexModelsTestAccount()
|
||||||
|
account.Credentials["refresh_token"] = "test-refresh-token"
|
||||||
|
|
||||||
|
_, err := s.FetchCodexModelsManifest(context.Background(), account, "0.137.0", "")
|
||||||
|
require.Error(t, err)
|
||||||
|
require.True(t, IsRetryableCodexModelsManifestError(err), "manifest 401 should allow account failover")
|
||||||
|
require.Equal(t, 1, repo.setTempUnschedCalls, "OAuth 401 should temp-unschedule the account")
|
||||||
|
require.Equal(t, 0, repo.setErrorCalls)
|
||||||
|
require.True(t, s.isOpenAIAccountRuntimeBlocked(account), "account should be runtime-blocked after manifest 401")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFetchCodexModelsManifestOAuth401TokenRevokedDisablesAccount(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusUnauthorized)
|
||||||
|
_, _ = w.Write([]byte(`{"error":{"code":"token_revoked","message":"token has been revoked"}}`))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
original := chatgptCodexModelsURL
|
||||||
|
chatgptCodexModelsURL = server.URL
|
||||||
|
defer func() { chatgptCodexModelsURL = original }()
|
||||||
|
|
||||||
|
repo := &codexModelsAccountStateRepo{}
|
||||||
|
s := newCodexModels401TestService(repo)
|
||||||
|
account := newCodexModelsTestAccount()
|
||||||
|
account.Credentials["refresh_token"] = "test-refresh-token"
|
||||||
|
|
||||||
|
_, err := s.FetchCodexModelsManifest(context.Background(), account, "0.137.0", "")
|
||||||
|
require.Error(t, err)
|
||||||
|
require.True(t, IsRetryableCodexModelsManifestError(err))
|
||||||
|
require.Equal(t, 1, repo.setErrorCalls, "revoked token should permanently disable the account")
|
||||||
|
require.Contains(t, repo.lastErrorMsg, "Token revoked")
|
||||||
|
require.Equal(t, 0, repo.setTempUnschedCalls)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFetchCodexModelsManifestAgentIdentity401DoesNotDisableAccount(t *testing.T) {
|
||||||
|
key, privateKey := newTestAgentIdentityKey(t)
|
||||||
|
account := &Account{
|
||||||
|
ID: 6,
|
||||||
|
Platform: PlatformOpenAI,
|
||||||
|
Type: AccountTypeOAuth,
|
||||||
|
Credentials: map[string]any{
|
||||||
|
"auth_mode": OpenAIAuthModeAgentIdentity,
|
||||||
|
"agent_runtime_id": key.runtimeID,
|
||||||
|
"agent_private_key": privateKey,
|
||||||
|
"task_id": key.taskID,
|
||||||
|
"chatgpt_account_id": "acc-agent-401",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusUnauthorized)
|
||||||
|
_, _ = w.Write([]byte(`{"detail":"some non-task 401"}`))
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
original := chatgptCodexModelsURL
|
||||||
|
chatgptCodexModelsURL = server.URL
|
||||||
|
defer func() { chatgptCodexModelsURL = original }()
|
||||||
|
|
||||||
|
repo := &codexModelsAccountStateRepo{}
|
||||||
|
s := newCodexModels401TestService(repo)
|
||||||
|
|
||||||
|
_, err := s.FetchCodexModelsManifest(context.Background(), account, "0.137.0", "")
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Equal(t, 0, repo.setErrorCalls, "agent identity 401s must not disable the account")
|
||||||
|
require.Equal(t, 0, repo.setTempUnschedCalls)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFetchCodexModelsManifestAPIKey401KeepsNoFailoverAndNoDisable(t *testing.T) {
|
||||||
|
upstream := &codexModelsHTTPUpstreamStub{do: func(_ *http.Request, _ string, _ int64, _ int) (*http.Response, error) {
|
||||||
|
return &http.Response{
|
||||||
|
StatusCode: http.StatusUnauthorized,
|
||||||
|
Status: "401 Unauthorized",
|
||||||
|
Header: make(http.Header),
|
||||||
|
Body: io.NopCloser(strings.NewReader(`{"error":"invalid api key"}`)),
|
||||||
|
}, nil
|
||||||
|
}}
|
||||||
|
|
||||||
|
repo := &codexModelsAccountStateRepo{}
|
||||||
|
s := newCodexModelsAPIKeyTestService(upstream)
|
||||||
|
s.rateLimitService = NewRateLimitService(repo, nil, &config.Config{}, nil, nil)
|
||||||
|
|
||||||
|
_, err := s.FetchCodexModelsManifest(
|
||||||
|
context.Background(),
|
||||||
|
newCodexModelsAPIKeyTestAccount("https://upstream.example"),
|
||||||
|
"0.144.0",
|
||||||
|
"",
|
||||||
|
)
|
||||||
|
require.Error(t, err)
|
||||||
|
require.False(t, IsRetryableCodexModelsManifestError(err), "custom upstream manifest 401 keeps the no-failover behavior")
|
||||||
|
require.Equal(t, 0, repo.setErrorCalls, "custom upstream manifest 401 must not disable the account")
|
||||||
|
require.Equal(t, 0, repo.setTempUnschedCalls)
|
||||||
|
}
|
||||||
|
|
||||||
func TestFetchCodexModelsManifestAPIKeyUpstreamError(t *testing.T) {
|
func TestFetchCodexModelsManifestAPIKeyUpstreamError(t *testing.T) {
|
||||||
upstream := &codexModelsHTTPUpstreamStub{do: func(_ *http.Request, _ string, _ int64, _ int) (*http.Response, error) {
|
upstream := &codexModelsHTTPUpstreamStub{do: func(_ *http.Request, _ string, _ int64, _ int) (*http.Response, error) {
|
||||||
return &http.Response{
|
return &http.Response{
|
||||||
|
|||||||
Reference in new issue
Block a user