From a32fdc8cfb2160771762ca07c62c30905a817d1d Mon Sep 17 00:00:00 2001 From: kaitranntt Date: Tue, 6 Jan 2026 13:05:45 -0500 Subject: [PATCH] fix(quota): address edge cases from code review - Add isPausingAccount disabled state to pause/resume dropdown (#30) - Add rapid click prevention guard in cliproxy.tsx (#31) - Add request deduplication via pendingFetches Map in quota-manager (#8) - Add JSON parse error handler middleware in web-server (#26) --- src/cliproxy/quota-manager.ts | 66 ++++++++++++------- src/web-server/index.ts | 16 ++++- .../cliproxy/provider-editor/account-item.tsx | 10 ++- .../provider-editor/accounts-section.tsx | 4 ++ .../cliproxy/provider-editor/index.tsx | 2 + .../provider-editor/model-config-tab.tsx | 4 ++ .../cliproxy/provider-editor/types.ts | 4 ++ ui/src/pages/cliproxy.tsx | 4 ++ 8 files changed, 81 insertions(+), 29 deletions(-) diff --git a/src/cliproxy/quota-manager.ts b/src/cliproxy/quota-manager.ts index 69a60005..abc09d80 100644 --- a/src/cliproxy/quota-manager.ts +++ b/src/cliproxy/quota-manager.ts @@ -36,6 +36,9 @@ interface CacheEntry { const CACHE_TTL_MS = 30_000; // 30 seconds const quotaCache = new Map(); +// Request deduplication: track in-flight fetch promises to avoid parallel duplicate requests +const pendingFetches = new Map>(); + function getCacheKey(provider: CLIProxyProvider, accountId: string): string { return `${provider}:${accountId}`; } @@ -76,6 +79,39 @@ export function clearQuotaCache(): void { quotaCache.clear(); } +/** + * Fetch quota with request deduplication + * If a fetch for this account is already in progress, return the existing promise + */ +async function fetchQuotaWithDedup( + provider: CLIProxyProvider, + accountId: string +): Promise { + const key = getCacheKey(provider, accountId); + + // Check if fetch already in progress + const pending = pendingFetches.get(key); + if (pending) { + return pending; + } + + // Start new fetch and track it + const fetchPromise = fetchAccountQuota(provider, accountId) + .then((result) => { + setCachedQuota(provider, accountId, result); + return result; + }) + .catch((): QuotaResult => { + return { success: false, models: [], lastUpdated: Date.now() }; + }) + .finally(() => { + pendingFetches.delete(key); + }); + + pendingFetches.set(key, fetchPromise); + return fetchPromise; +} + // ============================================================================ // COOLDOWN TRACKING // ============================================================================ @@ -176,17 +212,12 @@ export async function findHealthyAccount( if (available.length === 0) return null; - // Fetch quota for each available account (with caching) + // Fetch quota for each available account (with caching and deduplication) const withQuotas = await Promise.all( available.map(async (account) => { let quota = getCachedQuota(provider, account.id); if (!quota) { - try { - quota = await fetchAccountQuota(provider, account.id); - setCachedQuota(provider, account.id, quota); - } catch { - quota = { success: false, models: [], lastUpdated: Date.now() }; - } + quota = await fetchQuotaWithDedup(provider, account.id); } const avgQuota = calculateAverageQuota(quota); @@ -298,20 +329,10 @@ export async function preflightCheck(provider: CLIProxyProvider): Promise { let quota = getCachedQuota(provider, account.id); if (!quota && provider === 'agy') { - try { - quota = await fetchAccountQuota(provider, account.id); - setCachedQuota(provider, account.id, quota); - } catch { - quota = { success: false, models: [], lastUpdated: Date.now() }; - } + quota = await fetchQuotaWithDedup(provider, account.id); } const avgQuota = quota ? calculateAverageQuota(quota) : 100; diff --git a/src/web-server/index.ts b/src/web-server/index.ts index c5d4cfb2..5d43299c 100644 --- a/src/web-server/index.ts +++ b/src/web-server/index.ts @@ -32,8 +32,22 @@ export async function startServer(options: ServerOptions): Promise { + if (err instanceof SyntaxError && err.status === 400 && 'body' in err) { + res.status(400).json({ error: 'Invalid JSON in request body' }); + return; + } + next(err); + } + ); // REST API routes (modularized) const { apiRoutes } = await import('./routes/index'); diff --git a/ui/src/components/cliproxy/provider-editor/account-item.tsx b/ui/src/components/cliproxy/provider-editor/account-item.tsx index e12d1a29..072094ef 100644 --- a/ui/src/components/cliproxy/provider-editor/account-item.tsx +++ b/ui/src/components/cliproxy/provider-editor/account-item.tsx @@ -90,6 +90,7 @@ export function AccountItem({ onRemove, onPauseToggle, isRemoving, + isPausingAccount, privacyMode, showQuota, }: AccountItemProps) { @@ -188,16 +189,19 @@ export function AccountItem({ )} {onPauseToggle && ( - onPauseToggle(!account.paused)}> + onPauseToggle(!account.paused)} + disabled={isPausingAccount} + > {account.paused ? ( <> - Resume account + {isPausingAccount ? 'Resuming...' : 'Resume account'} ) : ( <> - Pause account + {isPausingAccount ? 'Pausing...' : 'Pause account'} )} diff --git a/ui/src/components/cliproxy/provider-editor/accounts-section.tsx b/ui/src/components/cliproxy/provider-editor/accounts-section.tsx index 2b566635..f9d43ed8 100644 --- a/ui/src/components/cliproxy/provider-editor/accounts-section.tsx +++ b/ui/src/components/cliproxy/provider-editor/accounts-section.tsx @@ -17,6 +17,8 @@ interface AccountsSectionProps { onRemoveAccount: (accountId: string) => void; onPauseToggle?: (accountId: string, paused: boolean) => void; isRemovingAccount?: boolean; + /** Pause/resume mutation in progress */ + isPausingAccount?: boolean; privacyMode?: boolean; /** Show quota bars for accounts (only applicable for 'agy' provider) */ showQuota?: boolean; @@ -34,6 +36,7 @@ export function AccountsSection({ onRemoveAccount, onPauseToggle, isRemovingAccount, + isPausingAccount, privacyMode, showQuota, isKiro, @@ -71,6 +74,7 @@ export function AccountsSection({ onPauseToggle ? (paused) => onPauseToggle(account.id, paused) : undefined } isRemoving={isRemovingAccount} + isPausingAccount={isPausingAccount} privacyMode={privacyMode} showQuota={showQuota} /> diff --git a/ui/src/components/cliproxy/provider-editor/index.tsx b/ui/src/components/cliproxy/provider-editor/index.tsx index 1cc552fa..b2d5b089 100644 --- a/ui/src/components/cliproxy/provider-editor/index.tsx +++ b/ui/src/components/cliproxy/provider-editor/index.tsx @@ -40,6 +40,7 @@ export function ProviderEditor({ onRemoveAccount, onPauseToggle, isRemovingAccount, + isPausingAccount, }: ProviderEditorProps) { const [customPresetOpen, setCustomPresetOpen] = useState(false); const { privacyMode } = usePrivacy(); @@ -203,6 +204,7 @@ export function ProviderEditor({ onRemoveAccount={onRemoveAccount} onPauseToggle={onPauseToggle} isRemovingAccount={isRemovingAccount} + isPausingAccount={isPausingAccount} privacyMode={privacyMode} isRemoteMode={isRemoteMode} /> diff --git a/ui/src/components/cliproxy/provider-editor/model-config-tab.tsx b/ui/src/components/cliproxy/provider-editor/model-config-tab.tsx index 25ca0306..51e46d55 100644 --- a/ui/src/components/cliproxy/provider-editor/model-config-tab.tsx +++ b/ui/src/components/cliproxy/provider-editor/model-config-tab.tsx @@ -38,6 +38,8 @@ interface ModelConfigTabProps { onRemoveAccount: (accountId: string) => void; onPauseToggle?: (accountId: string, paused: boolean) => void; isRemovingAccount?: boolean; + /** Pause/resume mutation in progress */ + isPausingAccount?: boolean; privacyMode?: boolean; /** True if connected to remote CLIProxy (quota not available) */ isRemoteMode?: boolean; @@ -63,6 +65,7 @@ export function ModelConfigTab({ onRemoveAccount, onPauseToggle, isRemovingAccount, + isPausingAccount, privacyMode, isRemoteMode, }: ModelConfigTabProps) { @@ -138,6 +141,7 @@ export function ModelConfigTab({ onRemoveAccount={onRemoveAccount} onPauseToggle={onPauseToggle} isRemovingAccount={isRemovingAccount} + isPausingAccount={isPausingAccount} privacyMode={privacyMode} showQuota={provider === 'agy' && !isRemoteMode} isKiro={isKiro} diff --git a/ui/src/components/cliproxy/provider-editor/types.ts b/ui/src/components/cliproxy/provider-editor/types.ts index 80010f9f..743a1bf3 100644 --- a/ui/src/components/cliproxy/provider-editor/types.ts +++ b/ui/src/components/cliproxy/provider-editor/types.ts @@ -32,6 +32,8 @@ export interface ProviderEditorProps { onRemoveAccount: (accountId: string) => void; onPauseToggle?: (accountId: string, paused: boolean) => void; isRemovingAccount?: boolean; + /** Pause/resume mutation in progress */ + isPausingAccount?: boolean; } export interface AccountItemProps { @@ -40,6 +42,8 @@ export interface AccountItemProps { onRemove: () => void; onPauseToggle?: (paused: boolean) => void; isRemoving?: boolean; + /** Pause/resume mutation in progress */ + isPausingAccount?: boolean; privacyMode?: boolean; /** Show quota bar (only for 'agy' provider) */ showQuota?: boolean; diff --git a/ui/src/pages/cliproxy.tsx b/ui/src/pages/cliproxy.tsx index bd13b9dc..52d2027e 100644 --- a/ui/src/pages/cliproxy.tsx +++ b/ui/src/pages/cliproxy.tsx @@ -221,6 +221,8 @@ export function CliproxyPage() { }; const handlePauseToggle = (provider: string, accountId: string, paused: boolean) => { + // Prevent rapid clicks while mutation is pending + if (pauseMutation.isPending || resumeMutation.isPending) return; if (paused) { pauseMutation.mutate({ provider, accountId }); } else { @@ -377,6 +379,7 @@ export function CliproxyPage() { handlePauseToggle(selectedVariantData.provider, accountId, paused) } isRemovingAccount={removeMutation.isPending} + isPausingAccount={pauseMutation.isPending || resumeMutation.isPending} /> ) : selectedStatus ? ( ) : ( setWizardOpen(true)} />