Repository navigation
fix(usage): allow quota refresh for FREE lease-reserved connections - #11758
Conversation
…mits refresh Usage and quota checks are read-only administrative operations that never mutate or disrupt active inference leases. Remove lease fences from providerLimits entirely so admin dashboard quota lookups always succeed.
9552c76 to
3004653
Compare
…ive-lease-usage-refresh-isolation
Keep model discovery, reset-credits, and other auxiliary paths fail-closed. Quota refresh is read-only admin telemetry and must proceed under an exclusive lease. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Thank you for identifying and fixing the quota-refresh regression. I agree that administrative quota and balance telemetry should not be blocked by lease reservation or active lease state, while credential-consuming auxiliary operations should remain fenced. I accept the technical miss and will apply this operation-classification distinction in future lease changes. For constructive collaboration, please keep the review focused on the code and avoid personal characterizations such as “AI slop” or “blind copy-pasting.” Those phrases do not improve the technical discussion. One factual note: the current implementation and tests allow quota refresh for both FREE and ACTIVE leased connections, while the PR title still mentions only FREE connections. Please update the title or summary to reflect the actual scope. |
|
Follow-up correction after reviewing the current implementation more closely: My previous comment was too broad in accepting live quota refresh for ACTIVE leases. The FREE lease-reservation regression is valid and should be fixed. However, the current PR removes the fence for ACTIVE leases as well and describes the operation as read-only. The manual usage route enables The current ACTIVE tests cover a DeepSeek API-key connection only; they do not cover Codex/OpenAI OAuth or a race between quota refresh and active inference. I recommend adjusting the implementation before merge:
Therefore, I support fixing the FREE regression, but I do not support the current blanket removal for ACTIVE leases without these safeguards. |
55691e0
into
diegosouzapw:release/v3.8.51
…iegosouzapw#11758) Boarded in a combined worktree with 6 other PRs: typecheck:core, check:dashboard-typecheck, check:file-size, check:changelog-integrity, check:complexity, check:cognitive-complexity, check:cycles, check-deps all green. Verified the root-cause diagnosis directly against the code: isConnectionUnavailableToAuxiliaryActivity() does return true for any connection reachable by an active exclusive lease regardless of whether the lease is actively serving a request, confirming the fix's scoping is correct. The change is surgically limited to providerLimits.ts's live-usage-fetch path — the shared isolation function and its other call sites (warmupScheduler, quotaAutoPing, modelTestRunner, etc.) are untouched. Well tested (214 lines across 3 test files). Thanks for tracking this down.
Problem & Root Cause
Introduced in commit
8acd799af(PR #10362) by @KaspaPulse:The exclusive managed session lease implementation introduced a severe regression that smells like unvetted AI slop — blindly copy-pasting an auxiliary isolation fence (
isConnectionUnavailableToAuxiliaryActivity) intosrc/lib/usage/providerLimits.tswithout understanding what the endpoint does.This caused a massive bug across the entire dashboard:
isConnectionUnavailableToAuxiliaryActivitystatically queries all active API keys withlease:exclusiveand treats any connection in theirallowedConnectionsas permanently unavailable. As a result, simply configuring or reserving connections instantly bricked the admin dashboard's live quota and balance refresh (GET /api/usage/{connectionId}) for every single provider connection 24/7, throwing HTTP 409 ("Usage refresh deferred while an exclusive lease is active"), even when the server was completely idle and no client held an active lease.GET https://api.deepseek.com/user/balance) is a harmless, read-only administrative call to upstream billing APIs. It does not touch model inference routes (/v1/chat/completions), does not mutate SQLite lease state, does not consume tokens from a conversation, and has zero impact on active inference sessions. Blocking admins from inspecting their own account balances is completely wrong.allowedConnectionson an API key is a client-level allowlist (restricting that specific key to certain connections). Commit8acd799afinverted this invariant into a global connection confiscation mechanism — treating an allowed connection as an exclusive private silo that starves full-permission API keys and administrative operations even when completely idle.Solution
fetchLiveProviderLimitsWithOptionsandsyncAllProviderLimitsinsrc/lib/usage/providerLimits.ts.Note: A follow-up PR will address the broader over-eager pool exclusion in
exclusiveConnectionLeasePolicy.tsso that full-access API keys are not erroneously starved of idle connections.cc @KaspaPulse — please thoroughly review and test AI-assisted code before shipping. Read-only administrative telemetry (like balance/quota lookups) must never be gated behind inference lease isolation.
Verification
Extended
tests/unit/exclusive-lease-auxiliary-isolation.test.tswith real database regressions:usage refresh allows FREE lease-reserved connection and queries provider quota: Verifies quota lookup succeeds on reserved connections.usage refresh allows ACTIVE leased connection and queries provider quota: Verifies quota lookup succeeds even when a connection actively has an active lease.syncAllProviderLimits refreshes all active supported connections regardless of lease state: Verifies bulk sync updates all active connections.All 10 tests in
exclusive-lease-auxiliary-isolation.test.tspassed.