From 54a2bcfd1580d0813d5ae9c8276cf7824e300a65 Mon Sep 17 00:00:00 2001 From: shaw Date: Mon, 3 Aug 2026 14:40:55 +0800 Subject: [PATCH] fix(openai): harden reset-credit refresh and account recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the reset-credit caching flow: - Recover account state BEFORE (and independently of) the reset-credit display cache. A failed cache refresh could previously abort the run and leave the account rate-limited — the very reason the credit was spent (#3672 / #3740). The recovered account row is now returned even when the cache refresh fails. - Run the post-reset bookkeeping on a detached, time-boxed context and give the panel reset call a larger timeout. A client abort no longer strands a consumed (non-refundable) credit with an unrecovered account, and the chained upstream calls can no longer exceed the client timeout and invite a retry that spends a second credit. - Persist the reset-credit snapshot through POST /accounts/:id/quota/refresh instead of a side-effecting GET flag, so the write is covered by the audit middleware. A rejected snapshot write now degrades to cache_persisted=false instead of turning a successful upstream read into a 502 that left the card without a credit count and the reset button permanently disabled. - Reject snapshots whose positive count carries no expiration entries, and drop expired credits (clamping the count) when rehydrating, so a stale cache can no longer light up the reset button. - Keep nil quota / rate-limit services nil in the handler's interface fields; storing a nil *Service made the "not enabled" guards non-nil. - Time-box the usage-refresh suppression and reuse handleAccountUpdated so the patched row also enters the auto-refresh silent window. --- .../handler/admin/openai_oauth_handler.go | 143 ++++-- .../openai_oauth_handler_reset_quota_test.go | 407 ++++++++++++++---- backend/internal/server/routes/admin.go | 1 + .../internal/service/openai_quota_service.go | 13 +- .../service/openai_quota_spark_window_test.go | 25 ++ frontend/src/api/admin/accounts.ts | 37 +- .../components/account/AccountUsageCell.vue | 12 +- .../account/OpenAIQuotaResetCell.vue | 55 ++- .../__tests__/AccountUsageCell.spec.ts | 4 +- .../OpenAIQuotaResetCell.spark_shadow.spec.ts | 179 +++++--- .../src/i18n/locales/en/admin/accounts.ts | 7 +- .../src/i18n/locales/zh/admin/accounts.ts | 7 +- frontend/src/types/index.ts | 4 + frontend/src/views/admin/AccountsView.vue | 2 +- 14 files changed, 681 insertions(+), 215 deletions(-) diff --git a/backend/internal/handler/admin/openai_oauth_handler.go b/backend/internal/handler/admin/openai_oauth_handler.go index 657eac4695..d7cfc8d2a6 100644 --- a/backend/internal/handler/admin/openai_oauth_handler.go +++ b/backend/internal/handler/admin/openai_oauth_handler.go @@ -40,6 +40,13 @@ const ( openAIQuotaResetWarningAccountRefreshFailed = "account_state_refresh_failed" ) +// openAIQuotaResetPostProcessTimeout bounds the work performed AFTER the +// (non-refundable) reset credit has already been consumed upstream. The whole +// request must stay comfortably inside the panel HTTP client timeout, otherwise +// the browser aborts a mutation that already succeeded and the operator retries +// it — spending a second credit. +const openAIQuotaResetPostProcessTimeout = 8 * time.Second + type openAIQuotaResetResponse struct { service.OpenAIQuotaResetResult Quota *service.OpenAIQuotaUsage `json:"quota,omitempty"` @@ -49,6 +56,27 @@ type openAIQuotaResetResponse struct { WarningCode string `json:"warning_code,omitempty"` } +// openAIQuotaRefreshResponse is the reset-credit-persisting variant of the quota +// query. The usage payload is embedded so the shape stays identical to the plain +// query; cache_persisted reports whether the snapshot write succeeded, because a +// failed display-cache write must never discard a successful upstream read. +type openAIQuotaRefreshResponse struct { + service.OpenAIQuotaUsage + CachePersisted bool `json:"cache_persisted"` +} + +// openAIQuotaResetPostProcessContext detaches the post-reset bookkeeping from the +// client connection. The credit is already spent at that point, so account-state +// recovery must complete even if the operator closes the tab (mirrors +// systemUpdateContext, added for the same reason in #4504). +func openAIQuotaResetPostProcessContext(ctx context.Context) (context.Context, context.CancelFunc) { + base := context.Background() + if ctx != nil { + base = context.WithoutCancel(ctx) + } + return context.WithTimeout(base, openAIQuotaResetPostProcessTimeout) +} + func oauthPlatformFromPath(c *gin.Context) string { return service.PlatformOpenAI } @@ -60,12 +88,20 @@ func NewOpenAIOAuthHandler( quotaService *service.OpenAIQuotaService, rateLimitService *service.RateLimitService, ) *OpenAIOAuthHandler { - return &OpenAIOAuthHandler{ + h := &OpenAIOAuthHandler{ openaiOAuthService: openaiOAuthService, adminService: adminService, - quotaService: quotaService, - rateLimitService: rateLimitService, } + // Assign through explicit nil checks: storing a nil *Service in an interface + // field yields a non-nil interface, which would silently defeat the + // `== nil` capability guards below and panic instead of returning 400. + if quotaService != nil { + h.quotaService = quotaService + } + if rateLimitService != nil { + h.rateLimitService = rateLimitService + } + return h } // OpenAIGenerateAuthURLRequest represents the request for generating OpenAI auth URL @@ -451,19 +487,8 @@ func (h *OpenAIOAuthHandler) QueryQuota(c *gin.Context) { response.BadRequest(c, "openai quota service is not enabled") return } - persistResetCredits := false - if raw, ok := c.GetQuery("persist_reset_credits"); ok { - persistResetCredits, err = strconv.ParseBool(raw) - if err != nil { - response.BadRequest(c, "Invalid persist_reset_credits value") - return - } - } usage, err := h.quotaService.QueryUsage(c.Request.Context(), accountID) - if err == nil && persistResetCredits { - err = h.quotaService.CacheResetCreditsSnapshot(c.Request.Context(), accountID, usage.RateLimitResetCredits) - } if err != nil { response.ErrorFrom(c, err) return @@ -471,6 +496,47 @@ func (h *OpenAIOAuthHandler) QueryQuota(c *gin.Context) { response.Success(c, usage) } +// RefreshQuota queries the rate-limit / quota usage AND persists the reset-credit +// snapshot so the card can be rehydrated without an upstream round-trip. +// POST /api/v1/admin/openai/accounts/:id/quota/refresh +// +// It is a POST (not a GET with a side-effect flag) because it writes account +// state: the audit middleware only records mutating verbs, so a persisting GET +// would mutate the database without an audit trail. +func (h *OpenAIOAuthHandler) RefreshQuota(c *gin.Context) { + accountID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + response.BadRequest(c, "Invalid account ID") + return + } + if h.quotaService == nil { + response.BadRequest(c, "openai quota service is not enabled") + return + } + + usage, err := h.quotaService.QueryUsage(c.Request.Context(), accountID) + if err != nil { + response.ErrorFrom(c, err) + return + } + if usage == nil { + response.Error(c, http.StatusInternalServerError, "openai quota query returned an empty result") + return + } + + refreshResponse := openAIQuotaRefreshResponse{OpenAIQuotaUsage: *usage} + // A failed snapshot write leaves the previous cache intact — report it as a + // partial success instead of discarding the usage payload we just fetched, + // which would leave the card without a credit count at all. + if err := h.quotaService.CacheResetCreditsSnapshot(c.Request.Context(), accountID, usage.RateLimitResetCredits); err != nil { + slog.Warn("openai_quota_reset_credit_cache_persist_failed", "account_id", accountID, "error", err) + response.Success(c, refreshResponse) + return + } + refreshResponse.CachePersisted = true + response.Success(c, refreshResponse) +} + // CreateShadowRequest is the request body for CreateShadow. type CreateShadowRequest struct { Name string `json:"name"` @@ -531,30 +597,22 @@ func (h *OpenAIOAuthHandler) ResetQuota(c *gin.Context) { } resetResponse := openAIQuotaResetResponse{OpenAIQuotaResetResult: *result} - usage, err := h.quotaService.QueryUsage(c.Request.Context(), accountID) - if err != nil { - slog.Warn("openai_quota_reset_cache_refresh_failed", "account_id", accountID, "error", err) - resetResponse.WarningCode = openAIQuotaResetWarningCacheRefreshFailed - response.Success(c, resetResponse) - return - } - if err := h.quotaService.CacheResetCreditsSnapshot(c.Request.Context(), accountID, usage.RateLimitResetCredits); err != nil { - slog.Warn("openai_quota_reset_cache_refresh_failed", "account_id", accountID, "error", err) - resetResponse.WarningCode = openAIQuotaResetWarningCacheRefreshFailed - response.Success(c, resetResponse) - return - } - resetResponse.Quota = usage - resetResponse.CacheRefreshed = true + postCtx, cancelPost := openAIQuotaResetPostProcessContext(c.Request.Context()) + defer cancelPost() + // Step 1 — unblocking the account is the whole point of consuming a credit + // (#3672 / #3740), so it runs FIRST and is never gated on the display cache. + // Recovery is DB-only and leaves the manual `schedulable` switch untouched. if h.rateLimitService == nil { resetResponse.WarningCode = openAIQuotaResetWarningAccountRecoveryFailed response.Success(c, resetResponse) return } - if _, err := h.rateLimitService.RecoverAccountState(c.Request.Context(), accountID, service.AccountRecoveryOptions{ + if _, err := h.rateLimitService.RecoverAccountState(postCtx, accountID, service.AccountRecoveryOptions{ InvalidateToken: true, }); err != nil { + // Recovery failures are almost always storage-level; the remaining steps + // share that dependency, so stop here instead of compounding the failure. slog.Warn("openai_quota_reset_account_recovery_failed", "account_id", accountID, "error", err) resetResponse.WarningCode = openAIQuotaResetWarningAccountRecoveryFailed response.Success(c, resetResponse) @@ -562,10 +620,31 @@ func (h *OpenAIOAuthHandler) ResetQuota(c *gin.Context) { } resetResponse.AccountStateRecovered = true - account, err := h.adminService.GetAccount(c.Request.Context(), accountID) + // Step 2 — refresh the reset-credit display cache. A failure here is reported + // but must not hide the recovered account row produced by step 3. + usage, usageErr := h.quotaService.QueryUsage(postCtx, accountID) + switch { + case usageErr != nil || usage == nil: + slog.Warn("openai_quota_reset_cache_refresh_failed", "account_id", accountID, "error", usageErr) + resetResponse.WarningCode = openAIQuotaResetWarningCacheRefreshFailed + default: + if err := h.quotaService.CacheResetCreditsSnapshot(postCtx, accountID, usage.RateLimitResetCredits); err != nil { + slog.Warn("openai_quota_reset_cache_refresh_failed", "account_id", accountID, "error", err) + resetResponse.WarningCode = openAIQuotaResetWarningCacheRefreshFailed + } else { + resetResponse.Quota = usage + resetResponse.CacheRefreshed = true + } + } + + // Step 3 — hand back the post-recovery account row so the list drops the + // stale rate-limit badge without waiting for the next poll. + account, err := h.adminService.GetAccount(postCtx, accountID) if err != nil { slog.Warn("openai_quota_reset_account_refresh_failed", "account_id", accountID, "error", err) - resetResponse.WarningCode = openAIQuotaResetWarningAccountRefreshFailed + if resetResponse.WarningCode == "" { + resetResponse.WarningCode = openAIQuotaResetWarningAccountRefreshFailed + } response.Success(c, resetResponse) return } diff --git a/backend/internal/handler/admin/openai_oauth_handler_reset_quota_test.go b/backend/internal/handler/admin/openai_oauth_handler_reset_quota_test.go index e287b32deb..c0e3ca161d 100644 --- a/backend/internal/handler/admin/openai_oauth_handler_reset_quota_test.go +++ b/backend/internal/handler/admin/openai_oauth_handler_reset_quota_test.go @@ -26,6 +26,9 @@ type openAIQuotaWorkflowStub struct { resetCalls int queryCalls int cacheCalls int + + queryCtxErr error + cacheCtxErr error } func (s *openAIQuotaWorkflowStub) ResetCredit(context.Context, int64) (*service.OpenAIQuotaResetResult, error) { @@ -33,13 +36,15 @@ func (s *openAIQuotaWorkflowStub) ResetCredit(context.Context, int64) (*service. return s.resetResult, s.resetErr } -func (s *openAIQuotaWorkflowStub) QueryUsage(context.Context, int64) (*service.OpenAIQuotaUsage, error) { +func (s *openAIQuotaWorkflowStub) QueryUsage(ctx context.Context, _ int64) (*service.OpenAIQuotaUsage, error) { s.queryCalls++ + s.queryCtxErr = ctx.Err() return s.queryResult, s.queryErr } -func (s *openAIQuotaWorkflowStub) CacheResetCreditsSnapshot(context.Context, int64, *service.OpenAIRateLimitResetCredits) error { +func (s *openAIQuotaWorkflowStub) CacheResetCreditsSnapshot(ctx context.Context, _ int64, _ *service.OpenAIRateLimitResetCredits) error { s.cacheCalls++ + s.cacheCtxErr = ctx.Err() return s.cacheErr } @@ -48,12 +53,14 @@ type openAIAccountStateRecovererStub struct { calls int accountID int64 lastOptions service.AccountRecoveryOptions + lastCtxErr error } -func (s *openAIAccountStateRecovererStub) RecoverAccountState(_ context.Context, accountID int64, options service.AccountRecoveryOptions) (*service.SuccessfulTestRecoveryResult, error) { +func (s *openAIAccountStateRecovererStub) RecoverAccountState(ctx context.Context, accountID int64, options service.AccountRecoveryOptions) (*service.SuccessfulTestRecoveryResult, error) { s.calls++ s.accountID = accountID s.lastOptions = options + s.lastCtxErr = ctx.Err() return &service.SuccessfulTestRecoveryResult{}, s.err } @@ -74,7 +81,19 @@ type openAIQuotaResetEnvelope struct { Data openAIQuotaResetResponse `json:"data"` } +type openAIQuotaRefreshEnvelope struct { + Code int `json:"code"` + Data openAIQuotaRefreshResponse `json:"data"` +} + func performOpenAIQuotaResetRequest(t *testing.T, handler *OpenAIOAuthHandler) (int, openAIQuotaResetEnvelope) { + t.Helper() + return performOpenAIQuotaResetRequestWithContext(t, handler, nil) +} + +// performOpenAIQuotaResetRequestWithContext drives the reset endpoint, optionally +// with an already-canceled request context (client disconnect simulation). +func performOpenAIQuotaResetRequestWithContext(t *testing.T, handler *OpenAIOAuthHandler, ctx context.Context) (int, openAIQuotaResetEnvelope) { t.Helper() gin.SetMode(gin.TestMode) @@ -82,6 +101,9 @@ func performOpenAIQuotaResetRequest(t *testing.T, handler *OpenAIOAuthHandler) ( router.POST("/api/v1/admin/openai/accounts/:id/reset-quota", handler.ResetQuota) recorder := httptest.NewRecorder() request := httptest.NewRequest(http.MethodPost, "/api/v1/admin/openai/accounts/42/reset-quota", nil) + if ctx != nil { + request = request.WithContext(ctx) + } router.ServeHTTP(recorder, request) var envelope openAIQuotaResetEnvelope @@ -89,6 +111,21 @@ func performOpenAIQuotaResetRequest(t *testing.T, handler *OpenAIOAuthHandler) ( return recorder.Code, envelope } +func performOpenAIQuotaRefreshRequest(t *testing.T, handler *OpenAIOAuthHandler) (int, openAIQuotaRefreshEnvelope) { + t.Helper() + gin.SetMode(gin.TestMode) + + router := gin.New() + router.POST("/api/v1/admin/openai/accounts/:id/quota/refresh", handler.RefreshQuota) + recorder := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodPost, "/api/v1/admin/openai/accounts/42/quota/refresh", nil) + router.ServeHTTP(recorder, request) + + var envelope openAIQuotaRefreshEnvelope + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &envelope)) + return recorder.Code, envelope +} + func successfulOpenAIQuotaWorkflowStub() *openAIQuotaWorkflowStub { return &openAIQuotaWorkflowStub{ resetResult: &service.OpenAIQuotaResetResult{ @@ -105,6 +142,17 @@ func successfulOpenAIQuotaWorkflowStub() *openAIQuotaWorkflowStub { } } +func recoveredAccountStub() *openAIResetAdminServiceStub { + return &openAIResetAdminServiceStub{account: &service.Account{ + ID: 42, + Name: "recovered", + Platform: service.PlatformOpenAI, + Type: service.AccountTypeOAuth, + Status: service.StatusActive, + Schedulable: false, + }} +} + func TestOpenAIResetQuota_ResetFailureStopsWorkflow(t *testing.T) { quota := &openAIQuotaWorkflowStub{resetErr: errors.New("upstream reset failed")} recoverer := &openAIAccountStateRecovererStub{} @@ -123,87 +171,12 @@ func TestOpenAIResetQuota_ResetFailureStopsWorkflow(t *testing.T) { require.Zero(t, recoverer.calls) } -func TestOpenAIResetQuota_QueryFailureReturnsPartialSuccessAndStops(t *testing.T) { - quota := successfulOpenAIQuotaWorkflowStub() - quota.queryResult = nil - quota.queryErr = errors.New("upstream query failed") - recoverer := &openAIAccountStateRecovererStub{} - handler := &OpenAIOAuthHandler{ - adminService: &openAIResetAdminServiceStub{}, - quotaService: quota, - rateLimitService: recoverer, - } - - status, envelope := performOpenAIQuotaResetRequest(t, handler) - - require.Equal(t, http.StatusOK, status) - require.Equal(t, openAIQuotaResetWarningCacheRefreshFailed, envelope.Data.WarningCode) - require.False(t, envelope.Data.CacheRefreshed) - require.False(t, envelope.Data.AccountStateRecovered) - require.Nil(t, envelope.Data.Quota) - require.Equal(t, 1, quota.resetCalls) - require.Equal(t, 1, quota.queryCalls) - require.Zero(t, quota.cacheCalls) - require.Zero(t, recoverer.calls) -} - -func TestOpenAIResetQuota_CacheFailureReturnsPartialSuccessAndStops(t *testing.T) { - quota := successfulOpenAIQuotaWorkflowStub() - quota.cacheErr = errors.New("cache write failed") - recoverer := &openAIAccountStateRecovererStub{} - handler := &OpenAIOAuthHandler{ - adminService: &openAIResetAdminServiceStub{}, - quotaService: quota, - rateLimitService: recoverer, - } - - status, envelope := performOpenAIQuotaResetRequest(t, handler) - - require.Equal(t, http.StatusOK, status) - require.Equal(t, openAIQuotaResetWarningCacheRefreshFailed, envelope.Data.WarningCode) - require.False(t, envelope.Data.CacheRefreshed) - require.Nil(t, envelope.Data.Quota) - require.Equal(t, 1, quota.resetCalls) - require.Equal(t, 1, quota.queryCalls) - require.Equal(t, 1, quota.cacheCalls) - require.Zero(t, recoverer.calls) -} - -func TestOpenAIResetQuota_RecoveryFailureKeepsRefreshedQuota(t *testing.T) { - quota := successfulOpenAIQuotaWorkflowStub() - recoverer := &openAIAccountStateRecovererStub{err: errors.New("recovery failed")} - adminService := &openAIResetAdminServiceStub{} - handler := &OpenAIOAuthHandler{ - adminService: adminService, - quotaService: quota, - rateLimitService: recoverer, - } - - status, envelope := performOpenAIQuotaResetRequest(t, handler) - - require.Equal(t, http.StatusOK, status) - require.Equal(t, openAIQuotaResetWarningAccountRecoveryFailed, envelope.Data.WarningCode) - require.True(t, envelope.Data.CacheRefreshed) - require.False(t, envelope.Data.AccountStateRecovered) - require.NotNil(t, envelope.Data.Quota) - require.Equal(t, 1, quota.resetCalls) - require.Equal(t, 1, quota.queryCalls) - require.Equal(t, 1, quota.cacheCalls) - require.Equal(t, 1, recoverer.calls) - require.Zero(t, adminService.calls) -} - -func TestOpenAIResetQuota_SuccessReturnsQuotaAndRecoveredAccount(t *testing.T) { +// Account-state recovery is the reason the credit was spent (#3672 / #3740), so it +// must run before — and independently of — the reset-credit display cache. +func TestOpenAIResetQuota_RecoversAccountStateBeforeRefreshingCache(t *testing.T) { quota := successfulOpenAIQuotaWorkflowStub() recoverer := &openAIAccountStateRecovererStub{} - adminService := &openAIResetAdminServiceStub{account: &service.Account{ - ID: 42, - Name: "recovered", - Platform: service.PlatformOpenAI, - Type: service.AccountTypeOAuth, - Status: service.StatusActive, - Schedulable: false, - }} + adminService := recoveredAccountStub() handler := &OpenAIOAuthHandler{ adminService: adminService, quotaService: quota, @@ -214,12 +187,12 @@ func TestOpenAIResetQuota_SuccessReturnsQuotaAndRecoveredAccount(t *testing.T) { require.Equal(t, http.StatusOK, status) require.Empty(t, envelope.Data.WarningCode) - require.True(t, envelope.Data.CacheRefreshed) require.True(t, envelope.Data.AccountStateRecovered) + require.True(t, envelope.Data.CacheRefreshed) require.NotNil(t, envelope.Data.Quota) require.NotNil(t, envelope.Data.Account) require.Equal(t, int64(42), envelope.Data.Account.ID) - require.False(t, envelope.Data.Account.Schedulable) + require.False(t, envelope.Data.Account.Schedulable, "manual scheduling switch must not be flipped") require.Equal(t, int64(42), recoverer.accountID) require.True(t, recoverer.lastOptions.InvalidateToken) require.Equal(t, 1, quota.resetCalls) @@ -229,6 +202,98 @@ func TestOpenAIResetQuota_SuccessReturnsQuotaAndRecoveredAccount(t *testing.T) { require.Equal(t, 1, adminService.calls) } +func TestOpenAIResetQuota_RecoveryFailureStopsWorkflow(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + recoverer := &openAIAccountStateRecovererStub{err: errors.New("recovery failed")} + adminService := recoveredAccountStub() + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + rateLimitService: recoverer, + } + + status, envelope := performOpenAIQuotaResetRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, openAIQuotaResetWarningAccountRecoveryFailed, envelope.Data.WarningCode) + require.False(t, envelope.Data.AccountStateRecovered) + require.False(t, envelope.Data.CacheRefreshed) + require.Nil(t, envelope.Data.Quota) + require.Nil(t, envelope.Data.Account) + require.Equal(t, 1, recoverer.calls) + require.Zero(t, quota.queryCalls) + require.Zero(t, quota.cacheCalls) + require.Zero(t, adminService.calls) +} + +func TestOpenAIResetQuota_MissingRecovererReportsRecoveryFailure(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + adminService := recoveredAccountStub() + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + } + + status, envelope := performOpenAIQuotaResetRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, openAIQuotaResetWarningAccountRecoveryFailed, envelope.Data.WarningCode) + require.False(t, envelope.Data.AccountStateRecovered) + require.Zero(t, quota.queryCalls) + require.Zero(t, adminService.calls) +} + +// A failed cache refresh must not hide the recovered account row: the operator +// still needs the list to drop the stale rate-limit badge. +func TestOpenAIResetQuota_QueryFailureStillRecoversAndReturnsAccount(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + quota.queryResult = nil + quota.queryErr = errors.New("upstream query failed") + recoverer := &openAIAccountStateRecovererStub{} + adminService := recoveredAccountStub() + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + rateLimitService: recoverer, + } + + status, envelope := performOpenAIQuotaResetRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, openAIQuotaResetWarningCacheRefreshFailed, envelope.Data.WarningCode) + require.True(t, envelope.Data.AccountStateRecovered) + require.False(t, envelope.Data.CacheRefreshed) + require.Nil(t, envelope.Data.Quota) + require.NotNil(t, envelope.Data.Account) + require.Equal(t, 1, quota.queryCalls) + require.Zero(t, quota.cacheCalls) + require.Equal(t, 1, recoverer.calls) + require.Equal(t, 1, adminService.calls) +} + +func TestOpenAIResetQuota_CacheFailureStillRecoversAndReturnsAccount(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + quota.cacheErr = errors.New("cache write failed") + recoverer := &openAIAccountStateRecovererStub{} + adminService := recoveredAccountStub() + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + rateLimitService: recoverer, + } + + status, envelope := performOpenAIQuotaResetRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, openAIQuotaResetWarningCacheRefreshFailed, envelope.Data.WarningCode) + require.True(t, envelope.Data.AccountStateRecovered) + require.False(t, envelope.Data.CacheRefreshed) + require.Nil(t, envelope.Data.Quota) + require.NotNil(t, envelope.Data.Account) + require.Equal(t, 1, quota.cacheCalls) + require.Equal(t, 1, adminService.calls) +} + func TestOpenAIResetQuota_AccountRefreshFailureReportsRecoveredState(t *testing.T) { quota := successfulOpenAIQuotaWorkflowStub() recoverer := &openAIAccountStateRecovererStub{} @@ -247,9 +312,175 @@ func TestOpenAIResetQuota_AccountRefreshFailureReportsRecoveredState(t *testing. require.True(t, envelope.Data.AccountStateRecovered) require.NotNil(t, envelope.Data.Quota) require.Nil(t, envelope.Data.Account) - require.Equal(t, 1, quota.resetCalls) - require.Equal(t, 1, quota.queryCalls) - require.Equal(t, 1, quota.cacheCalls) - require.Equal(t, 1, recoverer.calls) require.Equal(t, 1, adminService.calls) } + +// The first (most actionable) failure wins so the UI never downgrades a cache +// problem into a cosmetic "could not reload the row" message. +func TestOpenAIResetQuota_CacheAndAccountFailureKeepsFirstWarning(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + quota.cacheErr = errors.New("cache write failed") + recoverer := &openAIAccountStateRecovererStub{} + adminService := &openAIResetAdminServiceStub{err: errors.New("account refresh failed")} + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + rateLimitService: recoverer, + } + + status, envelope := performOpenAIQuotaResetRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, openAIQuotaResetWarningCacheRefreshFailed, envelope.Data.WarningCode) + require.True(t, envelope.Data.AccountStateRecovered) + require.Nil(t, envelope.Data.Account) +} + +// The credit is non-refundable once consumed, so post-reset bookkeeping must +// survive a client disconnect instead of leaving the account rate-limited. +func TestOpenAIResetQuota_PostProcessingSurvivesClientCancellation(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + recoverer := &openAIAccountStateRecovererStub{} + adminService := recoveredAccountStub() + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + rateLimitService: recoverer, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + status, _ := performOpenAIQuotaResetRequestWithContext(t, handler, ctx) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, 1, recoverer.calls) + require.NoError(t, recoverer.lastCtxErr, "recovery must not inherit the canceled client context") + require.NoError(t, quota.queryCtxErr) + require.NoError(t, quota.cacheCtxErr) + require.Equal(t, 1, adminService.calls) +} + +func TestOpenAIRefreshQuota_PersistsSnapshot(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + handler := &OpenAIOAuthHandler{ + adminService: &openAIResetAdminServiceStub{}, + quotaService: quota, + } + + status, envelope := performOpenAIQuotaRefreshRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.True(t, envelope.Data.CachePersisted) + require.Equal(t, int64(123), envelope.Data.FetchedAt) + require.Equal(t, 1, quota.queryCalls) + require.Equal(t, 1, quota.cacheCalls) + require.Zero(t, quota.resetCalls) +} + +// A rejected snapshot write must never discard the usage payload: otherwise the +// card loses its credit count and the reset button stays disabled forever. +func TestOpenAIRefreshQuota_PersistFailureStillReturnsUsage(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + quota.queryResult = &service.OpenAIQuotaUsage{ + FetchedAt: 456, + RateLimitResetCredits: &service.OpenAIRateLimitResetCredits{ + AvailableCount: 2, + }, + } + quota.cacheErr = errors.New("expiration details unavailable") + handler := &OpenAIOAuthHandler{ + adminService: &openAIResetAdminServiceStub{}, + quotaService: quota, + } + + status, envelope := performOpenAIQuotaRefreshRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.False(t, envelope.Data.CachePersisted) + require.Equal(t, int64(456), envelope.Data.FetchedAt) + require.NotNil(t, envelope.Data.RateLimitResetCredits) + require.Equal(t, 2, envelope.Data.RateLimitResetCredits.AvailableCount) + require.Equal(t, 1, quota.cacheCalls) +} + +// An empty-but-successful upstream read must not be dereferenced blindly. +func TestOpenAIQuotaEmptyUsageIsHandledWithoutPanic(t *testing.T) { + t.Run("refresh reports an internal error", func(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + quota.queryResult = nil + handler := &OpenAIOAuthHandler{ + adminService: &openAIResetAdminServiceStub{}, + quotaService: quota, + } + + status, envelope := performOpenAIQuotaRefreshRequest(t, handler) + + require.Equal(t, http.StatusInternalServerError, status) + require.False(t, envelope.Data.CachePersisted) + require.Zero(t, quota.cacheCalls) + }) + + t.Run("reset degrades to a cache warning", func(t *testing.T) { + quota := successfulOpenAIQuotaWorkflowStub() + quota.queryResult = nil + recoverer := &openAIAccountStateRecovererStub{} + adminService := recoveredAccountStub() + handler := &OpenAIOAuthHandler{ + adminService: adminService, + quotaService: quota, + rateLimitService: recoverer, + } + + status, envelope := performOpenAIQuotaResetRequest(t, handler) + + require.Equal(t, http.StatusOK, status) + require.Equal(t, openAIQuotaResetWarningCacheRefreshFailed, envelope.Data.WarningCode) + require.True(t, envelope.Data.AccountStateRecovered) + require.NotNil(t, envelope.Data.Account) + require.Zero(t, quota.cacheCalls) + }) +} + +func TestOpenAIRefreshQuota_QueryFailureIsReported(t *testing.T) { + quota := &openAIQuotaWorkflowStub{queryErr: errors.New("upstream query failed")} + handler := &OpenAIOAuthHandler{ + adminService: &openAIResetAdminServiceStub{}, + quotaService: quota, + } + + status, _ := performOpenAIQuotaRefreshRequest(t, handler) + + require.Equal(t, http.StatusInternalServerError, status) + require.Equal(t, 1, quota.queryCalls) + require.Zero(t, quota.cacheCalls) +} + +// Storing a nil *Service in an interface field would make the capability guards +// non-nil and panic on the first call; the constructor must keep them nil. +func TestNewOpenAIOAuthHandlerKeepsNilQuotaCapabilitiesGuarded(t *testing.T) { + gin.SetMode(gin.TestMode) + + handler := NewOpenAIOAuthHandler(nil, newStubAdminService(), nil, nil) + require.Nil(t, handler.quotaService) + require.Nil(t, handler.rateLimitService) + + router := gin.New() + router.Use(gin.Recovery()) + router.GET("/openai/accounts/:id/quota", handler.QueryQuota) + router.POST("/openai/accounts/:id/quota/refresh", handler.RefreshQuota) + router.POST("/openai/accounts/:id/reset-quota", handler.ResetQuota) + + for _, tc := range []struct { + method string + path string + }{ + {http.MethodGet, "/openai/accounts/42/quota"}, + {http.MethodPost, "/openai/accounts/42/quota/refresh"}, + {http.MethodPost, "/openai/accounts/42/reset-quota"}, + } { + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, httptest.NewRequest(tc.method, tc.path, nil)) + require.Equal(t, http.StatusBadRequest, recorder.Code, "%s %s", tc.method, tc.path) + } +} diff --git a/backend/internal/server/routes/admin.go b/backend/internal/server/routes/admin.go index a5635bec6e..161571cd53 100644 --- a/backend/internal/server/routes/admin.go +++ b/backend/internal/server/routes/admin.go @@ -437,6 +437,7 @@ func registerOpenAIOAuthRoutes(admin *gin.RouterGroup, h *handler.Handlers) { openai.POST("/create-from-oauth", h.Admin.OpenAIOAuth.CreateAccountFromOAuth) openai.POST("/create-from-codex-pat", h.Admin.OpenAIOAuth.CreateAccountFromCodexPAT) openai.GET("/accounts/:id/quota", h.Admin.OpenAIOAuth.QueryQuota) + openai.POST("/accounts/:id/quota/refresh", h.Admin.OpenAIOAuth.RefreshQuota) openai.POST("/accounts/:id/reset-quota", h.Admin.OpenAIOAuth.ResetQuota) } } diff --git a/backend/internal/service/openai_quota_service.go b/backend/internal/service/openai_quota_service.go index 82759f7011..58bf4659f5 100644 --- a/backend/internal/service/openai_quota_service.go +++ b/backend/internal/service/openai_quota_service.go @@ -208,9 +208,18 @@ func (s *OpenAIQuotaService) QueryUsage(ctx context.Context, accountID int64) (* } // CacheResetCreditsSnapshot persists a complete reset-credit snapshot after an -// explicit UI refresh. Missing expiration details leave the old cache intact. +// explicit UI refresh. The snapshot is written to the account that was queried +// (for a spark shadow that is the shadow row, even though the credits belong to +// its parent) because it is a per-row display cache: each row caches exactly +// what its own card renders, and shadows cannot consume credits anyway. +// +// Missing expiration details leave the old cache intact: +// a snapshot claiming N>0 available credits without their expiration timestamps +// cannot be aged out by readers, so it would keep showing (and offering to +// consume) credits that already expired. Callers must treat this rejection as a +// partial success — the upstream read itself is still valid. func (s *OpenAIQuotaService) CacheResetCreditsSnapshot(ctx context.Context, accountID int64, credits *OpenAIRateLimitResetCredits) error { - if credits == nil || (credits.AvailableCount > 0 && credits.Credits == nil) { + if credits == nil || (credits.AvailableCount > 0 && len(credits.Credits) == 0) { return infraerrors.New( http.StatusBadGateway, "OPENAI_QUOTA_RESET_CREDITS_REFRESH_FAILED", diff --git a/backend/internal/service/openai_quota_spark_window_test.go b/backend/internal/service/openai_quota_spark_window_test.go index 6182a04998..a351381cb4 100644 --- a/backend/internal/service/openai_quota_spark_window_test.go +++ b/backend/internal/service/openai_quota_spark_window_test.go @@ -605,6 +605,10 @@ func TestQueryUsageResetCreditDetails401NonFatal(t *testing.T) { require.Equal(t, 1, usage.RateLimitResetCredits.AvailableCount) require.Equal(t, 1, detailCalls) require.Empty(t, usage.RateLimitResetCredits.Credits) + + // A count without expiration details must not be persisted (the reader could + // never age it out), and the previous snapshot must survive untouched. + require.Error(t, svc.CacheResetCreditsSnapshot(ctx, 100, usage.RateLimitResetCredits)) require.Empty(t, repo.extraUpdates) } @@ -630,6 +634,27 @@ func TestCacheResetCreditsSnapshot(t *testing.T) { require.Empty(t, repo.extraUpdates) }) + t.Run("empty expiration list with a positive count preserves the cache", func(t *testing.T) { + repo := &stubQuotaAccountRepo{} + svc := &OpenAIQuotaService{accountRepo: repo} + + err := svc.CacheResetCreditsSnapshot(ctx, 100, &OpenAIRateLimitResetCredits{ + AvailableCount: 2, + Credits: []OpenAIRateLimitResetCreditDetail{}, + }) + + require.Error(t, err) + require.Empty(t, repo.extraUpdates) + }) + + t.Run("nil snapshot preserves the cache", func(t *testing.T) { + repo := &stubQuotaAccountRepo{} + svc := &OpenAIQuotaService{accountRepo: repo} + + require.Error(t, svc.CacheResetCreditsSnapshot(ctx, 100, nil)) + require.Empty(t, repo.extraUpdates) + }) + t.Run("repository errors are returned", func(t *testing.T) { repo := &stubQuotaAccountRepo{extraUpdateErr: errors.New("database unavailable")} svc := &OpenAIQuotaService{accountRepo: repo} diff --git a/frontend/src/api/admin/accounts.ts b/frontend/src/api/admin/accounts.ts index bab9b0c49a..bd9c54f8e1 100644 --- a/frontend/src/api/admin/accounts.ts +++ b/frontend/src/api/admin/accounts.ts @@ -844,24 +844,41 @@ export interface OpenAIQuotaResetResult { | 'account_state_refresh_failed' } +/** Usage payload plus whether the reset-credit snapshot was persisted. */ +export interface OpenAIQuotaRefreshResult extends OpenAIQuotaUsage { + cache_persisted: boolean +} + /** - * Query OpenAI/Codex rate-limit usage for an OAuth account. + * Query the upstream quota AND persist the reset-credit snapshot on the account + * so the card can be rehydrated without an upstream round-trip. It is a POST + * because it writes account state (and must therefore be audited). + * + * The read-only `GET /admin/openai/accounts/:id/quota` endpoint still exists for + * API consumers; the panel always wants the snapshot persisted, so it has no + * client binding here. */ -export async function queryOpenAIQuota( - id: number, - options?: { persistResetCredits?: boolean } -): Promise { - const { data } = await apiClient.get(`/admin/openai/accounts/${id}/quota`, { - params: options?.persistResetCredits === true ? { persist_reset_credits: true } : undefined - }) +export async function refreshOpenAIQuota(id: number): Promise { + const { data } = await apiClient.post( + `/admin/openai/accounts/${id}/quota/refresh` + ) return data } /** * Consume one rate-limit-reset credit for an OpenAI/Codex OAuth account. + * + * The credit is non-refundable and the endpoint chains an upstream reset with an + * upstream re-query, so it needs a larger budget than the default client + * timeout: aborting locally would report a successful consumption as a failure + * and invite a retry that spends a second credit. */ export async function resetOpenAIQuota(id: number): Promise { - const { data } = await apiClient.post(`/admin/openai/accounts/${id}/reset-quota`) + const { data } = await apiClient.post( + `/admin/openai/accounts/${id}/reset-quota`, + undefined, + { timeout: 90_000 } + ) return data } @@ -998,7 +1015,7 @@ export const accountsAPI = { batchRefresh, setPrivacy, revertProxyFallback, - queryOpenAIQuota, + refreshOpenAIQuota, resetOpenAIQuota, createSparkShadow, getUpstreamBillingProbeSettings, diff --git a/frontend/src/components/account/AccountUsageCell.vue b/frontend/src/components/account/AccountUsageCell.vue index 8be4361a39..bce09b37c8 100644 --- a/frontend/src/components/account/AccountUsageCell.vue +++ b/frontend/src/components/account/AccountUsageCell.vue @@ -643,6 +643,8 @@ import OllamaCloudUsageCell from './OllamaCloudUsageCell.vue' // Module-level cache shared across all AccountUsageCell instances const _usageCache = new Map() const USAGE_CACHE_TTL = 5 * 60 * 1000 // 5 minutes +// How long a quota-reset response may suppress the row-patch usage refetch. +const SUPPRESS_USAGE_REFRESH_WINDOW_MS = 5 * 1000 const props = withDefaults( defineProps<{ @@ -672,7 +674,7 @@ const loading = ref(false) const activeQueryLoading = ref(false) const error = ref(null) const usageInfo = ref(null) -const suppressNextOpenAIUsageRefresh = ref(false) +const suppressOpenAIUsageRefreshUntil = ref(0) const rootRef = ref(null) const isDesktopViewport = ref( typeof window === 'undefined' ? true : window.matchMedia(desktopViewportQuery).matches @@ -1485,7 +1487,9 @@ const quotaTotalBar = computed((): QuotaBarInfo | null => { const handleQuotaResetAccountUpdated = (account: Account) => { // The reset response already carries authoritative quota and account data. // Avoid turning the parent patch into a second automatic /usage request. - suppressNextOpenAIUsageRefresh.value = true + // The suppression is time-boxed so an unhandled emit (parent that ignores + // account-updated) cannot latch it and swallow a later, unrelated refresh. + suppressOpenAIUsageRefreshUntil.value = Date.now() + SUPPRESS_USAGE_REFRESH_WINDOW_MS emit('account-updated', account) } @@ -1533,8 +1537,8 @@ onMounted(() => { watch(openAIUsageRefreshKey, (nextKey, prevKey) => { if (!prevKey || nextKey === prevKey) return if (props.account.platform !== 'openai' || props.account.type !== 'oauth') return - if (suppressNextOpenAIUsageRefresh.value) { - suppressNextOpenAIUsageRefresh.value = false + if (Date.now() < suppressOpenAIUsageRefreshUntil.value) { + suppressOpenAIUsageRefreshUntil.value = 0 return } diff --git a/frontend/src/components/account/OpenAIQuotaResetCell.vue b/frontend/src/components/account/OpenAIQuotaResetCell.vue index dbaccd75cf..cb81bcfd9d 100644 --- a/frontend/src/components/account/OpenAIQuotaResetCell.vue +++ b/frontend/src/components/account/OpenAIQuotaResetCell.vue @@ -19,7 +19,7 @@ class="inline-flex items-center gap-0.5 rounded px-1.5 py-0.5 text-[10px] font-medium text-blue-600 transition-colors hover:bg-blue-50 disabled:cursor-not-allowed disabled:opacity-50 dark:text-blue-400 dark:hover:bg-blue-900/30" :disabled="loading || resetting" :title="countButtonTitle" - @click="handleQuery({ persistResetCredits: true })" + @click="handleQuery()" > (null) const showResetConfirm = ref(false) const showResetCreditDetails = ref(false) +// Rehydrate the card from the persisted snapshot. Credits that already expired +// are dropped and the count is clamped to what remains: the snapshot has no +// freshness signal, so an unfiltered read would offer to consume credits that no +// longer exist. A snapshot claiming credits with no usable expiration left is +// treated as absent, which keeps the reset button gated on a live query. const readCachedResetCredits = (account: Account): OpenAIQuotaUsage | null => { const cached = account.extra?.codex_reset_credit_snapshot if (!cached || typeof cached !== 'object' || Array.isArray(cached)) return null @@ -182,20 +187,28 @@ const readCachedResetCredits = (account: Account): OpenAIQuotaUsage | null => { } if (typeof count !== 'number' || !Number.isFinite(count)) return null + const now = Date.now() const credits: { expires_at?: string }[] = [] if (Array.isArray(rawCredits)) { for (const credit of rawCredits) { if (!credit || typeof credit !== 'object') continue const expiresAt = (credit as { expires_at?: unknown }).expires_at - if (typeof expiresAt === 'string' && expiresAt.trim() !== '') { - credits.push({ expires_at: expiresAt }) - } + if (typeof expiresAt !== 'string' || expiresAt.trim() === '') continue + const expiryTime = new Date(expiresAt).getTime() + // Unparsable timestamps are kept: they are already rendered verbatim and + // dropping them would silently understate the available count. + if (!Number.isNaN(expiryTime) && expiryTime <= now) continue + credits.push({ expires_at: expiresAt }) } } + const availableCount = Math.min(Math.max(count, 0), credits.length) + // A snapshot that claimed credits but has none left is no longer informative; + // report "unknown" so the operator re-queries instead of trusting it. + if (count > 0 && availableCount <= 0) return null return { fetched_at: 0, rate_limit_reset_credits: { - available_count: count, + available_count: availableCount, credits } } @@ -209,8 +222,11 @@ data.value = cachedData.value const isShadow = computed(() => props.account.parent_account_id != null) const availableResetCount = computed(() => data.value?.rate_limit_reset_credits?.available_count ?? 0) +// Prefer the live payload and fall back to the persisted snapshot only when the +// live state is unknown, so the count and the expirations never come from two +// different generations of the same data. const resetCreditExpirations = computed(() => - (cachedData.value?.rate_limit_reset_credits?.credits ?? []) + ((data.value ?? cachedData.value)?.rate_limit_reset_credits?.credits ?? []) .map((credit) => credit.expires_at?.trim() ?? '') .filter((expiresAt) => expiresAt.length > 0) .sort(compareResetCreditExpiry) @@ -304,7 +320,7 @@ const toggleResetCreditDetails = () => { showResetCreditDetails.value = !showResetCreditDetails.value } -const handleQuery = async (options?: { persistResetCredits?: boolean }) => { +const handleQuery = async () => { if (loading.value) return loading.value = true error.value = null @@ -312,11 +328,16 @@ const handleQuery = async (options?: { persistResetCredits?: boolean }) => { resetWarning.value = null showResetCreditDetails.value = false try { - const result = options - ? await queryOpenAIQuota(props.account.id, options) - : await queryOpenAIQuota(props.account.id) + const result = await refreshOpenAIQuota(props.account.id) + // The upstream read succeeded even when the snapshot write was rejected, so + // the live count is always adopted. Only the persisted view is left alone, + // which keeps the displayed expirations consistent with what is stored. data.value = result - if (options?.persistResetCredits) cachedData.value = result + if (result.cache_persisted) { + cachedData.value = result + } else { + resetWarning.value = t('admin.accounts.openaiQuotaReset.refreshCachePersistFailed') + } } catch (e) { error.value = extractErrorMessage(e) } finally { @@ -346,17 +367,19 @@ const confirmReset = async () => { resetWarning.value = null try { const result: OpenAIQuotaResetResult = await resetOpenAIQuota(props.account.id) + showResetCreditDetails.value = false if (result.cache_refreshed && result.quota) { data.value = result.quota cachedData.value = result.quota - showResetCreditDetails.value = false + } else { + // A credit was consumed but the post-reset count could not be read back. + // Whatever we still hold is one generation stale, so report the count as + // unknown instead of letting a second consumption start from stale data. + data.value = null } if (result.account) emit('account-updated', result.account) if (result.warning_code === 'reset_credit_cache_refresh_failed') { - // The persisted snapshot is intentionally preserved, but it is no longer - // authoritative enough to allow another credit consumption. - data.value = null resetWarning.value = t('admin.accounts.openaiQuotaReset.resetCacheRefreshFailed') } else if (result.warning_code === 'account_state_recovery_failed') { resetWarning.value = t('admin.accounts.openaiQuotaReset.resetAccountRecoveryFailed') diff --git a/frontend/src/components/account/__tests__/AccountUsageCell.spec.ts b/frontend/src/components/account/__tests__/AccountUsageCell.spec.ts index dd3e231879..6fcc963145 100644 --- a/frontend/src/components/account/__tests__/AccountUsageCell.spec.ts +++ b/frontend/src/components/account/__tests__/AccountUsageCell.spec.ts @@ -509,8 +509,8 @@ describe('AccountUsageCell', () => { await flushPromises() expect(getUsage).toHaveBeenCalledTimes(2) - expect(wrapper.text()).toContain('5h|0|200') - }) + expect(wrapper.text()).toContain('5h|0|200') + }) it('OpenAI 重置响应更新账号行时不会额外拉取 usage', async () => { getUsage.mockResolvedValue({ diff --git a/frontend/src/components/account/__tests__/OpenAIQuotaResetCell.spark_shadow.spec.ts b/frontend/src/components/account/__tests__/OpenAIQuotaResetCell.spark_shadow.spec.ts index b6182c9689..f888cbee44 100644 --- a/frontend/src/components/account/__tests__/OpenAIQuotaResetCell.spark_shadow.spec.ts +++ b/frontend/src/components/account/__tests__/OpenAIQuotaResetCell.spark_shadow.spec.ts @@ -3,10 +3,10 @@ import { flushPromises, mount } from '@vue/test-utils' import OpenAIQuotaResetCell from '../OpenAIQuotaResetCell.vue' import ConfirmDialog from '@/components/common/ConfirmDialog.vue' import type { Account } from '@/types' -import { queryOpenAIQuota, resetOpenAIQuota } from '@/api/admin/accounts' +import { refreshOpenAIQuota, resetOpenAIQuota } from '@/api/admin/accounts' vi.mock('@/api/admin/accounts', () => ({ - queryOpenAIQuota: vi.fn(), + refreshOpenAIQuota: vi.fn(), resetOpenAIQuota: vi.fn(), })) @@ -21,6 +21,11 @@ vi.mock('vue-i18n', async () => { } }) +// 缓存水合会丢弃已过期的重置卡，因此缓存类用例必须使用未来时间。 +const FUTURE_EXPIRY_EARLY = '2099-07-03T04:05:06Z' +const FUTURE_EXPIRY_LATE = '2099-07-05T04:05:06Z' +const PAST_EXPIRY = '2020-07-03T04:05:06Z' + function makeAccount(overrides: Partial): Account { return { id: 1, @@ -55,7 +60,7 @@ const resetButton = (wrapper: ReturnType) => wrapper.findAll('button')[1] beforeEach(() => { - vi.mocked(queryOpenAIQuota).mockReset() + vi.mocked(refreshOpenAIQuota).mockReset() vi.mocked(resetOpenAIQuota).mockReset() }) @@ -87,15 +92,15 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { codex_reset_credit_snapshot: { available_count: 2, credits: [ - { expires_at: '2026-07-05T04:05:06Z' }, - { expires_at: '2026-07-03T04:05:06Z' }, + { expires_at: FUTURE_EXPIRY_LATE }, + { expires_at: FUTURE_EXPIRY_EARLY }, ], }, }, }) const wrapper = mount(OpenAIQuotaResetCell, { props: { account } }) - expect(queryOpenAIQuota).not.toHaveBeenCalled() + expect(refreshOpenAIQuota).not.toHaveBeenCalled() expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.count') expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.expiresAt:') expect(wrapper.text()).toContain('+1') @@ -103,8 +108,48 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { wrapper.unmount() }) + it('缓存中的重置卡全部过期时视为未知,不点亮重置入口', () => { + const account = makeAccount({ + parent_account_id: null, + extra: { + codex_reset_credit_snapshot: { + available_count: 1, + credits: [{ expires_at: PAST_EXPIRY }], + }, + }, + }) + const wrapper = mount(OpenAIQuotaResetCell, { props: { account } }) + + expect(wrapper.text()).not.toContain('admin.accounts.openaiQuotaReset.expiresAt:') + const btn = resetButton(wrapper) + expect(btn.attributes('disabled')).toBeDefined() + expect(btn.attributes('title')).toBe('admin.accounts.openaiQuotaReset.resetTooltipNeedQuery') + wrapper.unmount() + }) + + it('缓存次数向未过期的明细数量收敛', () => { + const account = makeAccount({ + parent_account_id: null, + extra: { + codex_reset_credit_snapshot: { + available_count: 3, + credits: [ + { expires_at: PAST_EXPIRY }, + { expires_at: FUTURE_EXPIRY_EARLY }, + ], + }, + }, + }) + const wrapper = mount(OpenAIQuotaResetCell, { props: { account } }) + + expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.count1') + expect(wrapper.text()).not.toContain('+1') + expect(resetButton(wrapper).attributes('disabled')).toBeUndefined() + wrapper.unmount() + }) + it('查询后默认折叠为最早到期时间,点击 +N 展开完整列表', async () => { - vi.mocked(queryOpenAIQuota).mockResolvedValue({ + vi.mocked(refreshOpenAIQuota).mockResolvedValue({ rate_limit_reset_credits: { available_count: 3, credits: [ @@ -114,6 +159,7 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { ], }, fetched_at: 1770000000, + cache_persisted: true, }) const account = makeAccount({ parent_account_id: null }) @@ -122,7 +168,7 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { await wrapper.findAll('button')[0].trigger('click') await flushPromises() - expect(queryOpenAIQuota).toHaveBeenCalledWith(1, { persistResetCredits: true }) + expect(refreshOpenAIQuota).toHaveBeenCalledWith(1) expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.expiresAt:') expect(wrapper.text()).toContain('+2') expect(wrapper.text()).not.toContain('not-a-date') @@ -140,7 +186,7 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { }) it('只有一张重置卡时不显示展开按钮', async () => { - vi.mocked(queryOpenAIQuota).mockResolvedValue({ + vi.mocked(refreshOpenAIQuota).mockResolvedValue({ rate_limit_reset_credits: { available_count: 1, credits: [ @@ -148,6 +194,7 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { ], }, fetched_at: 1770000000, + cache_persisted: true, }) const account = makeAccount({ parent_account_id: null }) @@ -162,6 +209,27 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { wrapper.unmount() }) + // 快照写库被拒绝(上游未返回到期明细)不得吞掉这次成功的上游读取, + // 否则次数永远显示不出来、重置入口被永久禁用。 + it('快照持久化失败时仍显示实时次数并给出警告', async () => { + vi.mocked(refreshOpenAIQuota).mockResolvedValue({ + rate_limit_reset_credits: { available_count: 2 }, + fetched_at: 1770000000, + cache_persisted: false, + }) + + const account = makeAccount({ parent_account_id: null }) + const wrapper = mount(OpenAIQuotaResetCell, { props: { account } }) + + await wrapper.findAll('button')[0].trigger('click') + await flushPromises() + + expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.count2') + expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.refreshCachePersistFailed') + expect(resetButton(wrapper).attributes('disabled')).toBeUndefined() + wrapper.unmount() + }) + it('重置成功后直接使用响应中的最新缓存并回传恢复后的账号', async () => { const recoveredAccount = makeAccount({ parent_account_id: null, @@ -187,7 +255,7 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { extra: { codex_reset_credit_snapshot: { available_count: 1, - credits: [{ expires_at: '2026-07-03T04:05:06Z' }], + credits: [{ expires_at: FUTURE_EXPIRY_EARLY }], }, }, }) @@ -198,27 +266,68 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { await flushPromises() expect(resetOpenAIQuota).toHaveBeenCalledWith(1) - expect(queryOpenAIQuota).not.toHaveBeenCalled() + expect(refreshOpenAIQuota).not.toHaveBeenCalled() expect(wrapper.text()).not.toContain('admin.accounts.openaiQuotaReset.expiresAt:') expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.resetSuccess') expect(wrapper.emitted('account-updated')).toEqual([[recoveredAccount]]) wrapper.unmount() }) - it('缓存刷新失败时保留旧缓存并显示部分成功警告', async () => { + // 缓存回读失败不影响「账号状态已恢复」这一主目标:恢复后的账号行必须照常回传, + // 否则列表会继续显示已经不存在的限流状态。 + it('缓存刷新失败时仍回传恢复后的账号并把次数标为未知', async () => { + const recoveredAccount = makeAccount({ + parent_account_id: null, + status: 'active', + rate_limit_reset_at: null, + }) + vi.mocked(resetOpenAIQuota).mockResolvedValue({ + code: 'success', + windows_reset: 1, + cache_refreshed: false, + account_state_recovered: true, + warning_code: 'reset_credit_cache_refresh_failed', + account: recoveredAccount, + }) + const account = makeAccount({ + parent_account_id: null, + extra: { + codex_reset_credit_snapshot: { + available_count: 1, + credits: [{ expires_at: FUTURE_EXPIRY_EARLY }], + }, + }, + }) + const wrapper = mount(OpenAIQuotaResetCell, { props: { account } }) + + await resetButton(wrapper).trigger('click') + wrapper.findComponent(ConfirmDialog).vm.$emit('confirm') + await flushPromises() + + expect(refreshOpenAIQuota).not.toHaveBeenCalled() + // 次数未知(隐藏)但仍展示已持久化的到期明细,重置入口保持禁用直到重新查询。 + expect(wrapper.text()).not.toContain('admin.accounts.openaiQuotaReset.count1') + expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.expiresAt:') + expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.resetCacheRefreshFailed') + expect(resetButton(wrapper).attributes('disabled')).toBeDefined() + expect(wrapper.emitted('account-updated')).toEqual([[recoveredAccount]]) + wrapper.unmount() + }) + + it('账号状态恢复失败时停止后续步骤并提示手动恢复', async () => { vi.mocked(resetOpenAIQuota).mockResolvedValue({ code: 'success', windows_reset: 1, cache_refreshed: false, account_state_recovered: false, - warning_code: 'reset_credit_cache_refresh_failed', + warning_code: 'account_state_recovery_failed', }) const account = makeAccount({ parent_account_id: null, extra: { codex_reset_credit_snapshot: { available_count: 1, - credits: [{ expires_at: '2026-07-03T04:05:06Z' }], + credits: [{ expires_at: FUTURE_EXPIRY_EARLY }], }, }, }) @@ -228,48 +337,10 @@ describe('OpenAIQuotaResetCell — 外审 F6:影子禁用重置', () => { wrapper.findComponent(ConfirmDialog).vm.$emit('confirm') await flushPromises() - expect(queryOpenAIQuota).not.toHaveBeenCalled() - expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.expiresAt:') - expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.resetCacheRefreshFailed') + expect(refreshOpenAIQuota).not.toHaveBeenCalled() + expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.resetAccountRecoveryFailed') expect(resetButton(wrapper).attributes('disabled')).toBeDefined() expect(wrapper.emitted('account-updated')).toBeUndefined() wrapper.unmount() }) - - it('账号恢复失败时使用已刷新的缓存并显示部分成功警告', async () => { - vi.mocked(resetOpenAIQuota).mockResolvedValue({ - code: 'success', - windows_reset: 1, - cache_refreshed: true, - account_state_recovered: false, - warning_code: 'account_state_recovery_failed', - quota: { - rate_limit_reset_credits: { - available_count: 0, - credits: [], - }, - fetched_at: 1770000000, - }, - }) - const account = makeAccount({ - parent_account_id: null, - extra: { - codex_reset_credit_snapshot: { - available_count: 1, - credits: [{ expires_at: '2026-07-03T04:05:06Z' }], - }, - }, - }) - const wrapper = mount(OpenAIQuotaResetCell, { props: { account } }) - - await resetButton(wrapper).trigger('click') - wrapper.findComponent(ConfirmDialog).vm.$emit('confirm') - await flushPromises() - - expect(queryOpenAIQuota).not.toHaveBeenCalled() - expect(wrapper.text()).not.toContain('admin.accounts.openaiQuotaReset.expiresAt:') - expect(wrapper.text()).toContain('admin.accounts.openaiQuotaReset.resetAccountRecoveryFailed') - expect(wrapper.emitted('account-updated')).toBeUndefined() - wrapper.unmount() - }) }) diff --git a/frontend/src/i18n/locales/en/admin/accounts.ts b/frontend/src/i18n/locales/en/admin/accounts.ts index 5a910e3dda..283a33d386 100644 --- a/frontend/src/i18n/locales/en/admin/accounts.ts +++ b/frontend/src/i18n/locales/en/admin/accounts.ts @@ -1375,9 +1375,10 @@ export default { expirationDetails: 'Reset credit expiration details', noCreditsAvailable: 'No reset credits available', resetSuccess: 'Reset {windows} window(s); credits and account state updated', - resetCacheRefreshFailed: 'The window was reset, but the reset-credit cache refresh failed. Account state was not recovered.', - resetAccountRecoveryFailed: 'The window and reset-credit cache were updated, but account state recovery failed.', - resetAccountRefreshFailed: 'The window, reset-credit cache, and account state were updated, but the latest account display could not be loaded.', + resetCacheRefreshFailed: 'The window was reset and account state recovered, but the reset-credit count could not be read back. Query it again.', + resetAccountRecoveryFailed: 'The window was reset, but account state recovery failed. Recover the account state manually.', + resetAccountRefreshFailed: 'The window, account state, and reset-credit cache were updated, but the latest account display could not be loaded.', + refreshCachePersistFailed: 'Showing the live count, but its expiration details were unavailable, so the cached details were kept.', confirmTitle: 'Confirm Weekly Limit Reset', confirmMessage: 'This will consume 1 reset credit to immediately restore the current window ({count} remaining). This action cannot be undone. Continue?' }, diff --git a/frontend/src/i18n/locales/zh/admin/accounts.ts b/frontend/src/i18n/locales/zh/admin/accounts.ts index 43e81e7f50..63608cb8d4 100644 --- a/frontend/src/i18n/locales/zh/admin/accounts.ts +++ b/frontend/src/i18n/locales/zh/admin/accounts.ts @@ -430,9 +430,10 @@ export default { expirationDetails: '重置次数到期明细', noCreditsAvailable: '没有可用的重置次数', resetSuccess: '已重置 {windows} 个窗口，次数和账号状态已更新', - resetCacheRefreshFailed: '窗口已重置，但重置次数缓存刷新失败，账号状态未恢复。', - resetAccountRecoveryFailed: '窗口和重置次数缓存已更新，但账号状态恢复失败。', - resetAccountRefreshFailed: '窗口、重置次数缓存和账号状态已更新，但无法加载最新账号显示。', + resetCacheRefreshFailed: '窗口已重置、账号状态已恢复，但重置次数未能回读，请重新查询次数。', + resetAccountRecoveryFailed: '窗口已重置，但账号状态恢复失败，请手动恢复账号状态。', + resetAccountRefreshFailed: '窗口、账号状态和重置次数缓存已更新，但无法加载最新账号显示。', + refreshCachePersistFailed: '已显示实时次数，但到期明细获取失败，仍保留原有缓存明细。', confirmTitle: '确认重置周限', confirmMessage: '将消耗 1 次重置次数立即恢复当前窗口，剩余 {count} 次。此操作不可撤销，确定继续吗？' }, diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index cb0a698fb9..398e0c8b8b 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -1070,6 +1070,10 @@ export interface Account { upstream_billing_probe_enabled?: boolean upstream_billing_rate_sync_enabled?: boolean upstream_billing_probe?: UpstreamBillingProbeSnapshot + codex_reset_credit_snapshot?: { + available_count?: number + credits?: { expires_at?: string }[] + } } & Record) proxy_id: number | null proxy_fallback_origin_id?: number | null diff --git a/frontend/src/views/admin/AccountsView.vue b/frontend/src/views/admin/AccountsView.vue index 60a8b5ffa5..784f7c37f4 100644 --- a/frontend/src/views/admin/AccountsView.vue +++ b/frontend/src/views/admin/AccountsView.vue @@ -318,7 +318,7 @@ :today-stats="todayStatsByAccountId[String(row.id)] ?? null" :today-stats-loading="todayStatsLoading" :manual-refresh-token="usageManualRefreshToken" - @account-updated="patchAccountInList" + @account-updated="handleAccountUpdated" />