Conversation
新增或编辑供应商时,允许的模型列表优先从上游 API 获取真实可用的模型, 失败时回退到本地价格表,并通过图标指示数据来源(云端/本地)。
- 移除 wechat.ts 中无用的 case "info" 分支 - 修复 providers.ts 代码格式化问题
feat(provider): 允许的供应商模型支持从上游 API 动态获取
fix: 补全 Session 详情请求响应记录
- 添加条件确保 (durationMs - ttfbMs) >= 100ms - 移除 NULLIF 因为有了最小值检查后不再需要
fix: 修复 leaderboard 输出速率计算除以过小值的问题
…idation fix(redis): Redis Pub/Sub 跨进程缓存失效通知
Gemini CLI User-Agent "GeminiCLI/0.22.5/..." failed to match preset
pattern "gemini-cli" due to hyphen mismatch.
Changes:
- Add normalize function to remove hyphens/underscores before matching
- Skip empty patterns to prevent includes("") matching everything
- Add 24 unit tests covering edge cases and authState missing scenarios
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…exible feat: allow failureThreshold to be zero or above 100
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant enhancements to user and provider management, session observability, and system robustness. It refines user rate limiting by allowing nullable RPM and daily cost limits, improves session debugging capabilities by exposing more request/response metadata, and streamlines provider configuration by enabling dynamic fetching of models from upstream APIs. Additionally, it strengthens cache consistency across distributed environments using Redis Pub/Sub and adds a critical warning for circuit breaker configurations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to user and provider management, primarily focusing on flexible rate limiting and improved session debugging. Key changes include modifying the database schema to remove default values for rpm_limit and daily_limit_usd in the users table, allowing these limits to be explicitly null (unlimited). Corresponding UI updates were made across multiple locales (English, Japanese, Russian, Simplified Chinese, Traditional Chinese) to support 'RPM Limit' fields in user dashboards, batch editing, and limit rule pickers, including an 'unlimited' option. A new error message for RPM limit exceeding was also added. For providers, the circuit breaker failure threshold can now be set to 0 to disable it, with a confirmation dialog for special values. The ability to dynamically fetch model lists from upstream providers (OpenAI, Gemini, Anthropic) was added, complete with proxy support and API key handling. Session details were expanded to include requestBody, clientUrl, upstreamUrl, and statusCode for enhanced debugging. The cache invalidation mechanism for error rules was upgraded to use Redis Pub/Sub for better multi-worker synchronization. Additionally, the client guard logic for allowedClients was improved to normalize hyphens and underscores for more robust matching, and a minor bug fix was applied to the leaderboard's token per second calculation. A review comment highlighted the removal of executeWithRetry for session message storage, noting a potential decrease in reliability for non-critical logging, and another comment questioned the removal of specific translation keys in the Traditional Chinese dashboard locale.
I am having trouble creating individual review comments. Click here to see my feedback.
messages/zh-TW/dashboard.json (478-480)
It appears that the translation keys all, warning, and exceeded under the labels object have been removed. If these keys are still being used in the frontend for filtering or display purposes, this removal could lead to missing text or UI bugs for users of the Traditional Chinese locale. Please verify if these keys are still necessary. If they are obsolete, they should be removed from all other locale files as well to maintain consistency.
src/app/v1/_lib/proxy/session-guard.ts (91-97)
The executeWithRetry wrapper has been removed from the logic that stores session messages. The new implementation uses a "fire-and-forget" void ... .catch() pattern. While this correctly prevents blocking the main request flow, it also removes the resilience that the retry mechanism provided. If storing session messages for debugging is considered a critical feature, a transient Redis issue could now lead to data loss for that request.
If the reliability of this logging is important, I suggest reintroducing a lightweight retry mechanism. If it's considered a non-critical, "best-effort" operation, the current implementation is acceptable, but it's worth noting this change in reliability.
rpm=0 now means unlimited, so test negative value instead. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| "@/lib/redis/pubsub" | ||
| ); | ||
| await publishCacheInvalidation(CHANNEL_ERROR_RULES_UPDATED); | ||
| } catch { |
There was a problem hiding this comment.
[CRITICAL] [ERROR-SWALLOWED] Cache invalidation publish errors are silently ignored
Why this is a problem: The new catch {} swallows failures from publishCacheInvalidation, which violates the rule "No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect." If publish fails, other workers keep stale error rules with no diagnostic signal.
Suggested fix:
} catch (error) {
logger.warn("[emitErrorRulesUpdated] Failed to publish cache invalidation", { error });
}Add import { logger } from "@/lib/logger"; at the top of the file.
| "@/lib/redis/pubsub" | ||
| ); | ||
| await publishCacheInvalidation(CHANNEL_REQUEST_FILTERS_UPDATED); | ||
| } catch { |
There was a problem hiding this comment.
[CRITICAL] [ERROR-SWALLOWED] Cache invalidation publish errors are silently ignored
Why this is a problem: The new catch {} swallows failures from publishCacheInvalidation, which violates the rule "No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect." If publish fails, other workers keep stale request filters with no diagnostic signal.
Suggested fix:
} catch (error) {
logger.warn("[emitRequestFiltersUpdated] Failed to publish cache invalidation", { error });
}Add import { logger } from "@/lib/logger"; at the top of the file.
| CHANNEL_REQUEST_FILTERS_UPDATED, | ||
| handler | ||
| ); | ||
| } catch { |
There was a problem hiding this comment.
[CRITICAL] [ERROR-SWALLOWED] Redis pub/sub subscription failure is silently ignored
Why this is a problem: This new catch {} ignores errors from subscribeCacheInvalidation, violating the rule "No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect." If the subscription fails, cross-worker cache invalidation can silently stop and leave stale request filters.
Suggested fix:
} catch (error) {
logger.warn("[RequestFilterEngine] Failed to subscribe cache invalidation", { error });
}There was a problem hiding this comment.
Code Review Summary
XL-sized PR introducing new cache invalidation paths and model-fetch logic. Found three critical silent-failure cases in newly added Redis pub/sub publish/subscribe paths that can leave caches stale without any diagnostics. See inline comments for concrete fixes.
PR Size: XL
- Lines changed: 4305
- Files changed: 59
- Split suggestion: separate DB/schema + quota model changes, Redis pub/sub cache invalidation plumbing, UI/i18n updates, and upstream model fetch actions/tests.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 3 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- Silent swallow of pub/sub publish failure in
src/lib/emit-event.ts:25. - Silent swallow of pub/sub publish failure in
src/lib/emit-event.ts:62. - Silent swallow of pub/sub subscribe failure in
src/lib/request-filter-engine.ts:158.
High Priority Issues (Should Fix)
- None.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Summary
- Applied
size/XLlabel and posted the PR review summary. - Added 3 critical inline comments for silent error swallowing in new pub/sub paths (
src/lib/emit-event.ts:25,src/lib/emit-event.ts:62,src/lib/request-filter-engine.ts:158).
Next
- Fix the inline comments (log failures in the new
catch {}blocks). - Tell me when you want a re-review.
There was a problem hiding this comment.
Code Review Summary
This is a well-structured release PR (v0.3.40) with 59 changed files, implementing quota management improvements, Redis Pub/Sub for cross-process cache invalidation, dynamic provider model fetching, and several bug fixes. The code follows established patterns in the codebase and includes comprehensive test coverage for new functionality.
PR Size: XL
- Lines changed: 4,305 (4,081 additions, 224 deletions)
- Files changed: 59
Split Suggestion for Future: Consider separating large features into independent PRs (e.g., Redis Pub/Sub, quota management changes, dynamic model fetching) for easier review and safer rollback isolation.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Analysis
Redis Pub/Sub (src/lib/redis/pubsub.ts):
- Proper subscriber connection duplication (avoids blocking main Redis connection)
- Graceful degradation when Redis unavailable (silent failure, no exceptions)
- Error callbacks properly wrapped in try-catch
- Clean unsubscribe cleanup mechanism
Quota Management:
- Null-safe handling throughout the rate-limit-guard.ts (explicit
!== nullchecks) - Transformer layer properly normalizes zero values to null for unlimited semantics
- Type system correctly updated to
number | null - Test coverage added for normalization logic
Dynamic Model Fetching (src/actions/providers.ts):
- Proper authentication for all three provider types (Anthropic, OpenAI, Gemini)
- Timeout handling with AbortSignal
- Graceful fallback to local price table on failure
- Session validation before operations
Cache Invalidation:
- Dual notification pattern: local EventEmitter + Redis Pub/Sub
- Proper subscription cleanup tracked with cleanup functions
- Cross-worker synchronization properly implemented
Session Manager Enhancements:
- New metadata storage methods properly gated by
STORE_MESSAGESflag - Redis key TTL management consistent with existing patterns
- Proper sequence normalization
Error Handling Validation:
- All catch blocks in new code log errors appropriately
- Pub/Sub failures do not block operations (graceful degradation)
- Circuit breaker changes validated -
failureThreshold=0disabling is intentional design
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (proper graceful degradation patterns)
- Type safety - Clean (nullable types properly handled)
- Documentation accuracy - Clean
- Test coverage - Adequate (new test files for pubsub, transformers, client-guard)
- Code clarity - Good
Notes on Validation
Several potential issues were investigated but validated as non-issues:
-
Empty catch blocks in
emit-event.ts: These are intentional - the comment "忽略导入错误" (ignore import errors) indicates this is defensive coding for Edge Runtime compatibility. -
failureThreshold=0allowing circuit breaker disable: This is documented intentional behavior per the PR description for trusted providers. -
Error swallowing in
publishCacheInvalidation: This is intentional graceful degradation - cache invalidation failures should not block the primary operation.
Automated review by Claude AI
Summary
Release v0.3.40 merges 18 commits from the dev branch, including quota management improvements, Redis Pub/Sub cross-process cache invalidation, dynamic provider model fetching, and multiple bug fixes.
Related Issues:
Major Changes
1. RPM Management & Default Quota Removal (#499)⚠️ BREAKING
Problem:
Solution:
users.rpm_limitandusers.daily_limit_usd(Migration 0042)null(unlimited) at application runtimenumber | null)Breaking Changes:
nullinstead of 60/100Migration: Migration 0042 only affects new rows - existing users with RPM=60/Daily=$100 are not modified.
2. Redis Pub/Sub Cache Invalidation (#493)
Problem:
Solution:
src/lib/redis/pubsub.tswith publish/subscribe utilitiesImpact: Error rules and request filters now take effect immediately across all workers/instances (fixes #492).
3. Dynamic Provider Model Fetching (#491)
Problem:
Solution:
4. Circuit Breaker Flexibility (#498)
Problem:
failureThresholdwas rigidly restricted to 1-100 rangeSolution:
min(0)with no upper boundfailureThreshold = 0disables circuit breaking entirely5. Session Details Enhancements (#495)
Problem:
messagessubset, losing root-level fields (model,instructions)Solution:
requestBody+ request/response metadata (5-minute TTL)6. Leaderboard Calculation Fix (#497)
Problem:
durationMs - ttfbMswas very smallSolution:
(durationMs - ttfbMs) >= 100msto filter anomalous dataNULLIFwrapper7. Client Pattern Normalization
Problem:
Solution:
src/app/v1/_lib/proxy/client-guard.ts)Changes by Category
Database & Schema (2 files)
drizzle/0042_legal_harrier.sql- Remove defaults from users.rpm_limit and daily_limit_usdsrc/drizzle/schema.ts- Update schema to nullable without defaultsCore Services (12 files)
src/lib/redis/pubsub.ts(NEW) - Redis Pub/Sub utilitiessrc/lib/emit-event.ts- Added Redis Pub/Sub broadcastingsrc/lib/error-rule-detector.ts- Subscribe to Redis invalidationsrc/lib/request-filter-engine.ts- Subscribe to Redis invalidationsrc/lib/circuit-breaker.ts- Support failureThreshold=0src/lib/session-manager.ts- Store full requestBody + metadatasrc/app/v1/_lib/proxy/rate-limit-guard.ts- Handle nullable quotassrc/app/v1/_lib/proxy/session-guard.ts- Store complete request datasrc/app/v1/_lib/proxy/forwarder.ts- Record upstream metadatasrc/app/v1/_lib/proxy/client-guard.ts- Normalize hyphen/underscoresrc/repository/_shared/transformers.ts- Zero quota normalizationsrc/lib/validation/schemas.ts- Relaxed validation constraintsServer Actions (3 files)
src/actions/providers.ts- Dynamic model fetching from upstream APIssrc/actions/users.ts- Handle nullable quotassrc/actions/error-rules.ts- Broadcast via Redis Pub/Subsrc/actions/active-sessions.ts- Return full session detailsUI Components (14 files)
Testing (4 files)
src/lib/redis/__tests__/pubsub.test.ts(NEW) - Redis Pub/Sub testssrc/repository/_shared/transformers.test.ts(NEW) - Quota normalization teststests/unit/proxy/client-guard.test.ts(NEW) - Client pattern testssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsx- Session UI testsInternationalization (8 files)
Testing
Automated Tests
pubsub.test.ts,transformers.test.ts,client-guard.test.tsVerification Steps
Database Migration
Deployment Notes
Environment Variables (No changes required)
Existing env vars continue to work. Optional new features use existing Redis configuration.
Database Migration
Migration 0042 will run automatically if
AUTO_MIGRATE=true. This migration is safe - it only alters column defaults without modifying existing data.Multi-Worker Deployments
Redis Pub/Sub requires Redis to be configured. If Redis is not available, the system gracefully falls back to single-process behavior (existing behavior).
Breaking Changes Impact
For most deployments: No action required. New users will have unlimited quotas by default instead of RPM=60/Daily=$100.
If you relied on default quotas: Update user creation logic to explicitly set desired quota values.
TypeScript users: Check code that accesses
user.rpmoruser.dailyQuota- these are nownumber | null.Contributors
Checklist
Description enhanced by Claude AI
Greptile Summary
This release (v0.3.40) introduces flexible user quota management and enhanced multi-worker cache synchronization. The changes remove default rate limits, allowing users to have unlimited RPM and daily quotas by setting values to null.
Key Changes:
rpm_limitanddaily_limit_usd, allowing null to represent unlimited access. The database migration and schema were updated accordingly, and all user management flows now support this pattern.src/lib/redis/pubsub.ts) to synchronize cache invalidations across multiple worker processes/instances. Error rules and request filters now publish invalidation events to Redis when updated, ensuring all workers stay in sync.gemini-clinow matchesGeminiCLI,gemini_cli), improving compatibility with various CLI clients.failureThresholdcan now be set to 0 to disable the feature entirely, providing more deployment flexibility.fetchUpstreamModelsaction to retrieve model lists directly from OpenAI, Anthropic, and Gemini APIs with proxy support.Quality Improvements:
number | nullfor optional limitsConfidence Score: 5/5
Important Files Changed
rpmLimitanddailyLimitUsdnullable (no defaults)fetchUpstreamModelsfunction to retrieve model lists from OpenAI, Anthropic, and Gemini APIsfailureThresholdof 0 to disable circuit breakerGeminiCLIvsgemini-cliSequence Diagram
sequenceDiagram participant Client participant SessionGuard participant SessionManager participant RateLimitGuard participant Forwarder participant Redis participant Upstream Client->>SessionGuard: API Request SessionGuard->>SessionManager: getNextRequestSequence() SessionManager-->>SessionGuard: requestSequence SessionGuard->>SessionManager: storeSessionRequestBody() SessionGuard->>SessionManager: storeSessionClientRequestMeta() Note over SessionGuard,SessionManager: Store original request before processing SessionGuard->>RateLimitGuard: checkRateLimits() alt rpm is null Note over RateLimitGuard: Skip RPM check (unlimited) else rpm is set RateLimitGuard->>Redis: checkUserRPM() Redis-->>RateLimitGuard: allowed/denied end alt dailyQuota is null Note over RateLimitGuard: Skip daily quota check (unlimited) else dailyQuota is set RateLimitGuard->>Redis: checkUserDailyCost() Redis-->>RateLimitGuard: allowed/denied end RateLimitGuard-->>SessionGuard: Pass SessionGuard->>Forwarder: forward() Forwarder->>SessionManager: storeSessionUpstreamRequestMeta() Forwarder->>Upstream: HTTP Request Upstream-->>Forwarder: HTTP Response Forwarder->>SessionManager: storeSessionUpstreamResponseMeta() Forwarder-->>Client: Proxied Response Note over Redis: Multi-worker cache invalidation participant Admin Admin->>Redis: Update error rules/filters Redis->>Redis: Publish cache invalidation Redis-->>ErrorDetector: Redis Pub/Sub notification Redis-->>FilterEngine: Redis Pub/Sub notification ErrorDetector->>ErrorDetector: Reload cache FilterEngine->>FilterEngine: Reload cache