fix: address Gemini Code Review feedback on PR #2951

- Add !has(key) guard before eviction to avoid evicting entries
  that are about to be updated (combo.ts, apiKeyRotator.ts,
  codexQuotaFetcher.ts)
- Use optional chaining for provider?.toUpperCase() null safety
- Replace Object.values() with for-in in estimateSizeFast hot path
This commit is contained in:
soyelmismo
2026-05-30 13:03:02 -05:00
parent 31e11aa8d8
commit b4c0ce6519
4 changed files with 22 additions and 20 deletions

View File

@@ -293,8 +293,9 @@ function estimateSizeFast(value: unknown): number {
if (Array.isArray(v)) {
for (let i = 0; i < v.length; i++) stack.push(v[i]);
} else {
const vals = Object.values(v);
for (let i = 0; i < vals.length; i++) stack.push(vals[i]);
for (const key in v) {
if (Object.prototype.hasOwnProperty.call(v, key)) stack.push((v as Record<string, unknown>)[key]);
}
}
}
}
@@ -3997,7 +3998,7 @@ export async function handleChatCore({
(translatedBody.conversationState?.history?.length ?? 0) +
(translatedBody.conversationState?.currentMessage ? 1 : 0) ||
0;
log?.debug?.("REQUEST", `${provider.toUpperCase()} | ${model} | ${msgCount} msgs`);
log?.debug?.("REQUEST", `${provider?.toUpperCase()} | ${model} | ${msgCount} msgs`);
// ── Tier 2: Authoritative per-model/provider token-limit check (provider now resolved) ──
if (apiKeyInfo?.id) {
@@ -4233,7 +4234,7 @@ export async function handleChatCore({
};
if (newCredentials?.accessToken || newCredentials?.copilotToken) {
log?.info?.("TOKEN", `${provider.toUpperCase()} | refreshed`);
log?.info?.("TOKEN", `${provider?.toUpperCase()} | refreshed`);
// Fall back to post-mutex mutation only for executors that don't route
// through getAccessToken (and therefore never fire onPersist). For
@@ -4287,11 +4288,11 @@ export async function handleChatCore({
// than the original 401 alone. Surface at error level with sanitization.
log?.error?.(
"TOKEN",
`${provider.toUpperCase()} | retry after refresh failed: ${sanitizeErrorMessage(retryErr)}`
`${provider?.toUpperCase()} | retry after refresh failed: ${sanitizeErrorMessage(retryErr)}`
);
}
} else {
log?.warn?.("TOKEN", `${provider.toUpperCase()} | refresh failed`);
log?.warn?.("TOKEN", `${provider?.toUpperCase()} | refresh failed`);
if (isUnrecoverableRefreshError(newCredentials) && onCredentialsRefreshed) {
await onCredentialsRefreshed({ testStatus: "expired", isActive: false });
}
@@ -4942,7 +4943,7 @@ export async function handleChatCore({
const cacheUsageLogMeta = buildCacheUsageLogMeta(usage);
if (usage && typeof usage === "object") {
if (traceEnabled) {
const msg = `[${new Date().toLocaleTimeString("en-US", { hour12: false, hour: "2-digit", minute: "2-digit" })}] 📊 [USAGE] ${provider.toUpperCase()} | ${formatUsageLog(usage)}${connectionId ? ` | account=${connectionId.slice(0, 8)}...` : ""}`;
const msg = `[${new Date().toLocaleTimeString("en-US", { hour12: false, hour: "2-digit", minute: "2-digit" })}] 📊 [USAGE] ${provider?.toUpperCase()} | ${formatUsageLog(usage)}${connectionId ? ` | account=${connectionId.slice(0, 8)}...` : ""}`;
console.log(`${COLORS.green}${msg}${COLORS.reset}`);
}

View File

@@ -31,7 +31,7 @@ const MAX_CONNECTION_EXTRA_KEYS = 500;
*/
export function trackConnectionExtraKeys(connectionId: string, extraKeys: string[]): void {
const validExtras = extraKeys.filter((k) => typeof k === "string" && k.trim().length > 0);
if (_connectionExtraKeys.size >= MAX_CONNECTION_EXTRA_KEYS) {
if (!_connectionExtraKeys.has(connectionId) && _connectionExtraKeys.size >= MAX_CONNECTION_EXTRA_KEYS) {
const oldest = _connectionExtraKeys.keys().next().value;
if (oldest !== undefined) _connectionExtraKeys.delete(oldest);
}
@@ -276,11 +276,12 @@ export function syncHealthFromDB(connectionId: string, health?: Record<string, K
if (!health) return;
for (const [keyId, keyHealth] of Object.entries(health)) {
if (_keyHealth.size >= MAX_KEY_HEALTH_ENTRIES) {
const scopedKey = `${connectionId}:${keyId}`;
if (!_keyHealth.has(scopedKey) && _keyHealth.size >= MAX_KEY_HEALTH_ENTRIES) {
const oldest = _keyHealth.keys().next().value;
if (oldest !== undefined) _keyHealth.delete(oldest);
}
_keyHealth.set(`${connectionId}:${keyId}`, keyHealth);
_keyHealth.set(scopedKey, keyHealth);
}
}

View File

@@ -86,7 +86,7 @@ const MAX_QUOTA_CACHE_ENTRIES = 200;
* @param meta - Access token and optional workspace ID
*/
export function registerCodexConnection(connectionId: string, meta: CodexConnectionMeta): void {
if (connectionRegistry.size >= MAX_CONNECTIONS) {
if (!connectionRegistry.has(connectionId) && connectionRegistry.size >= MAX_CONNECTIONS) {
const oldestKey = connectionRegistry.keys().next().value;
if (oldestKey !== undefined) {
quotaCache.delete(oldestKey);
@@ -124,7 +124,7 @@ function getCodexConnectionMeta(
if (accessToken) {
const meta = { accessToken, ...(workspaceId ? { workspaceId } : {}) };
if (connectionRegistry.size >= MAX_CONNECTIONS) {
if (!connectionRegistry.has(connectionId) && connectionRegistry.size >= MAX_CONNECTIONS) {
const oldestKey = connectionRegistry.keys().next().value;
if (oldestKey !== undefined) {
quotaCache.delete(oldestKey);
@@ -212,7 +212,7 @@ export async function fetchCodexQuota(
if (!quota) return null;
// Store in cache
if (quotaCache.size >= MAX_QUOTA_CACHE_ENTRIES) {
if (!quotaCache.has(connectionId) && quotaCache.size >= MAX_QUOTA_CACHE_ENTRIES) {
const oldestCacheKey = quotaCache.keys().next().value;
if (oldestCacheKey !== undefined) quotaCache.delete(oldestCacheKey);
}

View File

@@ -1550,7 +1550,7 @@ async function getQuotaAwareConnectionsForTarget(
const activeConnections = Array.isArray(connections)
? (connections as Array<Record<string, unknown>>)
: [];
if (resetAwareConnectionCache.size >= MAX_RESET_AWARE_CACHE) {
if (!resetAwareConnectionCache.has(provider) && resetAwareConnectionCache.size >= MAX_RESET_AWARE_CACHE) {
const oldest = resetAwareConnectionCache.keys().next().value;
if (oldest !== undefined) resetAwareConnectionCache.delete(oldest);
}
@@ -1685,7 +1685,7 @@ async function fetchResetAwareQuotaWithCache({
const refreshPromise = fetcher(connectionId, connection)
.then((quota) => {
if (quota) {
if (resetAwareQuotaCache.size >= MAX_RESET_AWARE_CACHE) {
if (!resetAwareQuotaCache.has(cacheKey) && resetAwareQuotaCache.size >= MAX_RESET_AWARE_CACHE) {
const oldest = resetAwareQuotaCache.keys().next().value;
if (oldest !== undefined) resetAwareQuotaCache.delete(oldest);
}
@@ -1702,7 +1702,7 @@ async function fetchResetAwareQuotaWithCache({
.catch((error) => {
const previous = resetAwareQuotaCache.get(cacheKey);
if (previous) {
if (resetAwareQuotaCache.size >= MAX_RESET_AWARE_CACHE) {
if (!resetAwareQuotaCache.has(cacheKey) && resetAwareQuotaCache.size >= MAX_RESET_AWARE_CACHE) {
const oldest = resetAwareQuotaCache.keys().next().value;
if (oldest !== undefined) resetAwareQuotaCache.delete(oldest);
}
@@ -1718,7 +1718,7 @@ async function fetchResetAwareQuotaWithCache({
return null;
});
if (resetAwareQuotaCache.size >= MAX_RESET_AWARE_CACHE) {
if (!resetAwareQuotaCache.has(cacheKey) && resetAwareQuotaCache.size >= MAX_RESET_AWARE_CACHE) {
const oldest = resetAwareQuotaCache.keys().next().value;
if (oldest !== undefined) resetAwareQuotaCache.delete(oldest);
}
@@ -1845,7 +1845,7 @@ async function orderTargetsByResetAwareQuota(
if (tiedTargets.length > 1) {
const key = `reset-aware:${comboName}`;
const counter = rrCounters.get(key) || 0;
if (rrCounters.size >= MAX_RR_COUNTERS) {
if (!rrCounters.has(key) && rrCounters.size >= MAX_RR_COUNTERS) {
const oldest = rrCounters.keys().next().value;
if (oldest !== undefined) rrCounters.delete(oldest);
}
@@ -2008,7 +2008,7 @@ async function orderTargetsByResetWindow(
const key = `reset-window:${comboName}`;
const counter = rrCounters.get(key) || 0;
if (rrCounters.size >= MAX_RR_COUNTERS) {
if (!rrCounters.has(key) && rrCounters.size >= MAX_RR_COUNTERS) {
const oldest = rrCounters.keys().next().value;
if (oldest !== undefined) rrCounters.delete(oldest);
}
@@ -3921,7 +3921,7 @@ async function handleRoundRobinCombo({
// Get and increment atomic counter
const counter = rrCounters.get(combo.name) || 0;
if (rrCounters.size >= MAX_RR_COUNTERS) {
if (!rrCounters.has(combo.name) && rrCounters.size >= MAX_RR_COUNTERS) {
const oldest = rrCounters.keys().next().value;
if (oldest !== undefined) rrCounters.delete(oldest);
}