Repository navigation
Improve self-service provider quota visibility - #2931
diegosouzapw merged 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the self-service API key status endpoint to support returning multiple provider account quotas (accountQuotas) instead of restricting the output to a single connection or returning an ambiguity error. It updates BDD features, specifications, and unit tests to reflect this change, including support for unrestricted keys and isolated per-connection fetch failures. Additionally, it ensures backwards compatibility by retaining the single accountQuota field when only one quota is returned. In the UI, it updates the permissions modal to use i18n keys and explicit button types. Feedback on the code changes suggests improving resilience by adding .catch handlers to the connection retrieval calls (getProviderConnectionById and getProviderConnections) to prevent potential database or retrieval failures from causing a 500 error.
| const rawConnections = | ||
| allowedConnections.length > 0 | ||
| ? await Promise.all(allowedConnections.map((id) => deps.getProviderConnectionById(id))) | ||
| : await deps.getProviderConnections({ isActive: true }); |
There was a problem hiding this comment.
To ensure the self-service status API is resilient against individual connection fetch failures or database query rejections, consider adding .catch(() => null) to getProviderConnectionById and .catch(() => []) to getProviderConnections. This prevents a single failing connection from causing the entire request to fail with a 500 error.
const rawConnections =
allowedConnections.length > 0
? await Promise.all(
allowedConnections.map((id) =>
deps.getProviderConnectionById(id).catch(() => null)
)
)
: await deps.getProviderConnections({ isActive: true }).catch(() => []);4c09cbc to
76528a5
Compare
|
Addressed the review feedback on connection retrieval resilience in the latest push. Explicit connection lookup failures now return a per-connection unavailable entry with |
76528a5 to
472001b
Compare
|
Also addressed the Unit Tests 4/8 CI failure seen on the previous run. The failing chatCore timeout test used a hard-coded 150ms pending-request polling window; under the full shard load it could observe no pending detail before asserting. The latest push uses the existing wait helper default and the exact CI shard command now passes locally: 906/906. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The changes in this PR are well-implemented and thoroughly tested. Key improvements:
Observation (unchanged code): The Ban Toggle button (line ~1901) in the PermissionsModal does not have Files Reviewed (5 files)
Reviewed by nemotron-3-super-120b-a12b-20230311:free · 121,871 tokens |
472001b to
8f0c97a
Compare
|
Addressed the E2E Tests 2/6 failure from the previous run. The create-key modal now keeps validation errors reachable after the new self-service controls make the dialog taller: on invalid names it scrolls/focuses back to the key name field and the modal body has a viewport-aware max height. Verified locally with the exact failing shard command: |
8f0c97a to
54cec88
Compare
|
Follow-up for the latest E2E Tests 2/6 timeout: the cancelled job was blocked on a Local verification after the patch:
|
|
Thank you for your contribution! This PR has been reviewed and integrated into the upcoming |
Integrated into release/v3.8.8
Integrated into release/v3.8.8
Integrated into release/v3.8.8
Summary
GET /api/v1/me/statusaccount quota visibility from the single Codex case to all provider-limit connections the API key is allowed to use.accountQuotasfor multi-provider quota entries while preserving the existingaccountQuotacompatibility field when exactly one entry is returned.Behavior Notes
self:account-quotaremains opt-in and disabled by default.allowedConnectionsare resolved one by one; unrestricted keys enumerate all active provider-limit connections because they may use all of them.connection_lookup_failedentry; unrestricted connection-list failures keep own usage visible with an emptyaccountQuotasarray.Verification
DISABLE_SQLITE_AUTO_BACKUP=true node --max-old-space-size=8192 --import tsx --import ./open-sse/utils/setupPolyfill.ts --test --test-force-exit tests/unit/api-key-self-service.test.ts tests/unit/api-manager-page-static.test.ts tests/unit/api-manager-scope-preservation.test.ts tests/unit/api-key-scope-validation.test.ts tests/unit/api/v1-me-status-route.test.ts tests/unit/budget-route-auth.test.ts— 22/22 passnode --max-old-space-size=4096 --import tsx --test --test-force-exit --test-concurrency=4 --test-shard=4/8 tests/unit/*.test.ts— 906/906 passnpm run typecheck:core— passnpm run lint -- --quiet— passnpm run check:docs-sync— passnpm run i18n:sync-ui:dry && npm run i18n:check-ui-coverage— pass; dry-run reports existing missing locale keys, coverage passesgit diff --check && rg -n "sk-4bf65f1c839de16d|100\.68\.118\.98|guanzhicheng\.com" docs src tests || true— pass/no matchesnpm run build— pass