fix: provider total limit editable + enforce + reset#535
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthrough本PR为提供商引入“总消费上限”支持:新增数据库字段、后端限额检查扩展、仓库与统计函数、管理端表单与列表按钮、国际化文案以及对应测试用例和迁移快照。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting 📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx,js,jsx,json}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/lib/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*provider*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.{tsx,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{tsx,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-01-03T09:08:20.573ZApplied to files:
📚 Learning: 2026-01-03T09:08:49.019ZApplied to files:
🧬 Code graph analysis (2)src/app/[locale]/settings/providers/_components/forms/provider-form.tsx (3)
tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (11)
Comment |
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! 此拉取请求旨在全面修复供应商总消费限额功能,使其在前端可见、可编辑,并在后端得到严格的端到端校验和管理。通过引入新的数据库字段、更新前端界面、实现新的后端业务逻辑以及增加全面的测试,确保了供应商总消费限额的有效性、可控性和用户体验。 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.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx (1)
597-611: UI 按钮渲染逻辑正确,视觉区分清晰。总用量重置按钮的实现很好:
- 条件渲染逻辑正确(仅当
limitTotalUsd > 0时显示)- 使用蓝色图标与熔断重置的橙色图标区分开来
- pending 状态正确禁用按钮防止重复点击
- 事件传播已正确阻止(
e.stopPropagation())可选的小优化建议:可以考虑在按钮上添加
aria-label属性以提升可访问性:🔎 可访问性增强建议
<Button size="icon" variant="ghost" title={tList("resetUsageTitle")} + aria-label={tList("resetUsageTitle")} onClick={(e) => {tests/unit/proxy/provider-selector-total-limit.test.ts (1)
45-65: 测试数据构造可考虑使用工厂函数简化。两个测试用例中的 provider 对象构造重复度较高,且使用
as unknown as Provider类型断言。建议考虑抽取一个createMockProvider工厂函数来减少重复并提高可维护性。不过,当前实现功能正确,这只是一个可选的优化建议。
🔎 建议的重构方案
function createMockProvider(overrides: Partial<Provider> = {}): Provider { return { id: 1, name: "p1", isEnabled: true, providerType: "claude", groupTag: null, weight: 1, priority: 0, costMultiplier: 1, limit5hUsd: null, limitDailyUsd: null, dailyResetMode: "fixed", dailyResetTime: "00:00", limitWeeklyUsd: null, limitMonthlyUsd: null, limitTotalUsd: null, totalCostResetAt: null, limitConcurrentSessions: 0, ...overrides, } as unknown as Provider; }Also applies to: 66-85
tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx (2)
188-194: 重试循环处理 React transition 时序问题是合理的做法。由于 React 的 transition 可能延后调度 action 调用,使用重试循环等待
editProvider被调用是一种务实的解决方案。不过,可以考虑使用waitFor工具来替代手动重试循环,使代码更具可读性。🔎 可选优化:使用 waitFor 替代手动重试
如果项目中已引入
@testing-library/react,可以使用其waitFor工具:import { waitFor } from "@testing-library/react"; // 替代手动重试循环 await waitFor(() => { expect(providersActionMocks.editProvider).toHaveBeenCalledTimes(1); });
112-156: 测试数据完整但建议避免as any类型断言。provider 对象涵盖了所有必要字段,但使用
as any类型断言会绕过类型检查。建议使用更精确的类型或部分类型来保持类型安全。🔎 可选优化:使用 ProviderDisplay 类型
- } as any; + } as import("@/types/provider").ProviderDisplay;或者补充缺失的可选字段使其完全符合类型定义。
src/lib/rate-limit/service.ts (1)
78-83: 总消费限额扩展到 provider 路径逻辑基本 OK,可微调 key 模式与导入方式整体来看,这里把
checkTotalCostLimit扩展到"provider":
- provider 路径会:
- 使用 cache key
total_cost:provider:${entityId}:${resetAtMs};- DB 回退时通过
sumProviderTotalCost(entityId, options?.resetAt ?? null),且 resetAt 会被有效性检查,手动重置后会自然切换到新 cache key,语义正确。- user/path 也复用
sumUserTotalCost,逻辑合理。有两个地方可以小幅优化一下:
- key 模式下 keyHash 缺失时,建议更早 return,避免生成伪缓存 key
当前实现中,即使
entityType === "key"且没有传options?.keyHash,也会先生成:const cacheKey = `total_cost:key:${options?.keyHash}`; // => total_cost:key:undefined然后再在 DB 分支里检查:
if (!options?.keyHash) { logger.warn("[RateLimit] Missing key hash for total cost check, skip enforcement"); return { allowed: true }; }为了避免出现
total_cost:key:undefined这类无语义的缓存 key,建议在函数开头直接短路返回:建议重构:缺少 keyHash 时尽早短路
static async checkTotalCostLimit( entityId: number, entityType: "key" | "user" | "provider", limitTotalUsd: number | null, options?: { keyHash?: string; resetAt?: Date | null } ): Promise<{ allowed: boolean; current?: number; reason?: string }> { if (limitTotalUsd === null || limitTotalUsd === undefined || limitTotalUsd <= 0) { return { allowed: true }; } + + // key 模式缺少 keyHash 时直接放行,避免生成 total_cost:key:undefined 这类伪 key + if (entityType === "key" && !options?.keyHash) { + logger.warn("[RateLimit] Missing key hash for total cost check, skip enforcement"); + return { allowed: true }; + } @@ - const cacheKey = (() => { - if (entityType === "key") { - return `total_cost:key:${options?.keyHash}`; - } + const cacheKey = (() => { + if (entityType === "key") { + // 此时已保证 options.keyHash 存在 + return `total_cost:key:${options!.keyHash}`; + }这样可以保证所有缓存 key 都对应真实实体标识,也更易于排查问题。
- 统计函数导入方式略显冗余(可选)
顶部已经静态导入了:
import { sumKeyTotalCost, sumProviderTotalCost, sumUserCostInTimeRange, sumUserTotalCost, } from "@/repository/statistics";而在
checkCostLimitsFromDatabase/getCurrentCost/checkUserDailyCost中,又通过动态import("@/repository/statistics")解构了同一个sumUserCostInTimeRange等函数。功能上没问题,但依赖关系稍显重复。可以考虑二选一:
- 要么完全依赖静态导入(移除动态导入中的同名函数);
- 要么在所有使用点都改成动态导入,移除顶部静态导入。
这只是依赖整理/加载策略的风格问题,不影响正确性。
另外,
typeName对 provider 使用"供应商",组合出的英文 reason 为供应商 total spending limit reached (...),和上面其它中文 reason 略有风格差异,如果后续统一国际化策略时可以顺手调整。Also applies to: 280-305, 317-326, 336-345, 349-358, 361-367
src/actions/providers.ts (1)
722-748: 实现良好,建议考虑添加审计日志。
resetProviderTotalUsage函数实现正确:
- 正确验证 admin 权限
- 调用 repository 层重置
totalCostResetAt- 正确处理供应商不存在的情况
- 触发相关路径的 revalidation
建议:考虑在成功重置后添加
logger.info记录操作,包含userId和providerId,以便审计追踪(类似getUnmaskedProviderKey函数的做法)。🔎 建议添加审计日志
const ok = await resetProviderTotalCostResetAt(providerId, new Date()); if (!ok) { return { ok: false, error: "供应商不存在" }; } + logger.info("Admin reset provider total usage", { + userId: session.user.id, + providerId, + }); + revalidatePath("/settings/providers"); revalidatePath("/dashboard/quotas/providers"); return { ok: true };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (23)
drizzle/0045_mushy_human_torch.sqldrizzle/meta/0045_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/settings.jsonmessages/ja/settings.jsonmessages/ru/settings.jsonmessages/zh-CN/settings.jsonmessages/zh-TW/settings.jsonsrc/actions/providers.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.tsxsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/drizzle/schema.tssrc/lib/rate-limit/service.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/provider.tssrc/repository/statistics.tssrc/types/provider.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/provider-selector-total-limit.test.tstests/unit/settings/providers/provider-form-total-limit-ui.test.tsx
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation in all code files
Files:
src/drizzle/schema.tsdrizzle/meta/_journal.jsonsrc/repository/_shared/transformers.tssrc/lib/validation/schemas.tssrc/repository/statistics.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/settings/providers/provider-form-total-limit-ui.test.tsxsrc/repository/provider.tsmessages/ja/settings.jsonsrc/lib/rate-limit/service.tsmessages/zh-CN/settings.jsontests/unit/proxy/provider-selector-total-limit.test.tsdrizzle/meta/0045_snapshot.jsonsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/actions/providers.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.tsxmessages/zh-TW/settings.jsonsrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.tsmessages/en/settings.jsonmessages/ru/settings.json
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use double quotes for strings instead of single quotes
Use trailing commas in multi-line structures
Enforce maximum line length of 100 characters
Use path alias@/*to reference files from./src/*directory
**/*.{ts,tsx,js,jsx}: Use Biome for linting and formatting with 2-space indent, double quotes, trailing commas, and 100 character max line length
Use path alias@/*to reference files in./src/*directory
Files:
src/drizzle/schema.tssrc/repository/_shared/transformers.tssrc/lib/validation/schemas.tssrc/repository/statistics.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/settings/providers/provider-form-total-limit-ui.test.tsxsrc/repository/provider.tssrc/lib/rate-limit/service.tstests/unit/proxy/provider-selector-total-limit.test.tssrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/actions/providers.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.tsxsrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.ts
src/drizzle/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM with PostgreSQL for database operations
src/drizzle/**/*.ts: Use Drizzle ORM with parameterized queries to prevent SQL injection
Use soft delete pattern withdeletedAtcolumn instead of hard deletes
Use JSON columns in PostgreSQL for flexible data structures (modelRedirects, providerChain, etc.)
Implement proper indexing strategy for common queries and foreign keys
Files:
src/drizzle/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode for type safety
Use readonly or const assertions for immutable data structures
Files:
src/drizzle/schema.tssrc/repository/_shared/transformers.tssrc/lib/validation/schemas.tssrc/repository/statistics.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/settings/providers/provider-form-total-limit-ui.test.tsxsrc/repository/provider.tssrc/lib/rate-limit/service.tstests/unit/proxy/provider-selector-total-limit.test.tssrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/actions/providers.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.tsxsrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Hash API keys using SHA-256 before storing in database, never store plaintext keys
Mask API keys and sensitive data in application logs
Validate required environment variables at startup with clear error messages
Files:
src/drizzle/schema.tssrc/repository/_shared/transformers.tssrc/lib/validation/schemas.tssrc/repository/statistics.tssrc/repository/provider.tssrc/lib/rate-limit/service.tssrc/actions/providers.tssrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.ts
**/*.{tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use next-intl for internationalization with 5 locales: en, ja, ru, zh-CN, zh-TW
Files:
drizzle/meta/_journal.jsontests/unit/settings/providers/provider-form-total-limit-ui.test.tsxmessages/ja/settings.jsonmessages/zh-CN/settings.jsondrizzle/meta/0045_snapshot.jsonsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form.tsxmessages/zh-TW/settings.jsonmessages/en/settings.jsonmessages/ru/settings.json
src/repository/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Repository pattern in
src/repository/to wrap Drizzle queries
Files:
src/repository/_shared/transformers.tssrc/repository/statistics.tssrc/repository/provider.ts
src/{repository,actions}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Avoid N+1 queries by using eager loading and batch queries for statistics
Files:
src/repository/_shared/transformers.tssrc/repository/statistics.tssrc/repository/provider.tssrc/actions/providers.ts
src/lib/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use connection pooling for database and Redis connections
Files:
src/lib/validation/schemas.tssrc/lib/rate-limit/service.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Files:
tests/unit/lib/rate-limit/cost-limits.test.tstests/unit/settings/providers/provider-form-total-limit-ui.test.tsxtests/unit/proxy/provider-selector-total-limit.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Ensure test database names contain 'test' keyword for safety validation
Files:
tests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/provider-selector-total-limit.test.ts
src/**/*provider*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Set provider circuit breaker failure threshold, open duration, and half-open success threshold in configuration
Files:
src/repository/provider.tssrc/actions/providers.tssrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.ts
messages/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
Support 5 locales via next-intl: en, ja, ru, zh-CN, zh-TW with messages in
messages/{locale}/*.jsonStore message translations in
messages/{locale}/*.jsonfiles
Files:
messages/ja/settings.jsonmessages/zh-CN/settings.jsonmessages/zh-TW/settings.jsonmessages/en/settings.jsonmessages/ru/settings.json
src/lib/rate-limit/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Rate Limiting must track dimensions: RPM, cost (5h/week/month), concurrent sessions at User, Key, and Provider levels using Redis Lua scripts
Use Redis with Lua scripts for atomic rate limiting operations
Files:
src/lib/rate-limit/service.ts
src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{tsx,jsx}: Uselucide-reactfor icons, no custom SVGs
Use React's automatic escaping to prevent XSS vulnerabilities
Files:
src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form.tsx
src/actions/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/actions/**/*.ts: Validate all user inputs with Zod schemas before processing
Use Server Actions innext-safe-actionwith OpenAPI generation for admin API endpoints
Use Next.js API Routes and Server Actions for admin operations and REST endpoints
Files:
src/actions/providers.ts
src/app/v1/_lib/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Guard pipeline must execute in order: ProxyAuthenticator, SensitiveWordGuard, VersionGuard, ProxySessionGuard, ProxyRateLimitGuard, ProxyProviderResolver
Files:
src/app/v1/_lib/proxy/rate-limit-guard.tssrc/app/v1/_lib/proxy/provider-selector.ts
src/app/v1/_lib/proxy/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/app/v1/_lib/proxy/**/*.ts: Implement guard pipeline pattern for cross-cutting concerns in request processing (auth, rate limiting, session)
Use undici library for HTTP requests instead of node-fetch for better performance
Files:
src/app/v1/_lib/proxy/rate-limit-guard.tssrc/app/v1/_lib/proxy/provider-selector.ts
src/**/*{guard,auth}*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use constant-time comparison for API key validation to prevent timing attacks
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/app/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement Content-Security-Policy headers for XSS prevention
Files:
src/app/v1/_lib/proxy/rate-limit-guard.tssrc/app/v1/_lib/proxy/provider-selector.ts
src/app/v1/_lib/proxy/**/*guard*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement fail-open strategy: allow requests when Redis is unavailable
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/app/v1/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Hono router for ultrafast, lightweight routing in proxy endpoints
Files:
src/app/v1/_lib/proxy/rate-limit-guard.tssrc/app/v1/_lib/proxy/provider-selector.ts
🧠 Learnings (11)
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/drizzle/**/*.ts : Use JSON columns in PostgreSQL for flexible data structures (modelRedirects, providerChain, etc.)
Applied to files:
src/drizzle/schema.tsdrizzle/meta/0045_snapshot.json
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to src/lib/rate-limit/**/*.ts : Rate Limiting must track dimensions: RPM, cost (5h/week/month), concurrent sessions at User, Key, and Provider levels using Redis Lua scripts
Applied to files:
src/drizzle/schema.tstests/unit/lib/rate-limit/cost-limits.test.tssrc/repository/provider.tssrc/lib/rate-limit/service.tstests/unit/proxy/provider-selector-total-limit.test.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.tsxmessages/zh-TW/settings.jsonsrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/actions/**/*.ts : Validate all user inputs with Zod schemas before processing
Applied to files:
src/lib/validation/schemas.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/**/*{message,response,log}*.ts : Log request duration, token usage, and cost to message_request table for analytics
Applied to files:
src/repository/statistics.tssrc/lib/rate-limit/service.tssrc/app/v1/_lib/proxy/rate-limit-guard.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/lib/rate-limit/**/*.ts : Use Redis with Lua scripts for atomic rate limiting operations
Applied to files:
tests/unit/lib/rate-limit/cost-limits.test.tssrc/lib/rate-limit/service.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/**/*provider*.ts : Set provider circuit breaker failure threshold, open duration, and half-open success threshold in configuration
Applied to files:
tests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/provider-selector-total-limit.test.tssrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/types/provider.tssrc/app/v1/_lib/proxy/provider-selector.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Applied to files:
tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to messages/**/*.json : Support 5 locales via next-intl: en, ja, ru, zh-CN, zh-TW with messages in `messages/{locale}/*.json`
Applied to files:
messages/ja/settings.jsonmessages/zh-CN/settings.jsonmessages/zh-TW/settings.jsonmessages/en/settings.jsonmessages/ru/settings.json
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/app/v1/_lib/proxy/**/*.ts : Implement guard pipeline pattern for cross-cutting concerns in request processing (auth, rate limiting, session)
Applied to files:
tests/unit/proxy/provider-selector-total-limit.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/app/v1/_lib/proxy/provider-selector.ts
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to src/app/v1/_lib/**/*.ts : Guard pipeline must execute in order: ProxyAuthenticator, SensitiveWordGuard, VersionGuard, ProxySessionGuard, ProxyRateLimitGuard, ProxyProviderResolver
Applied to files:
src/app/v1/_lib/proxy/rate-limit-guard.tssrc/app/v1/_lib/proxy/provider-selector.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/**/*{guard,auth}*.ts : Use constant-time comparison for API key validation to prevent timing attacks
Applied to files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
🧬 Code graph analysis (7)
src/repository/statistics.ts (2)
src/drizzle/db.ts (1)
db(37-44)src/drizzle/schema.ts (1)
messageRequest(266-347)
tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx (1)
src/app/[locale]/settings/providers/_components/forms/provider-form.tsx (1)
ProviderForm(54-1870)
src/repository/provider.ts (1)
src/drizzle/schema.ts (1)
providers(129-263)
src/lib/rate-limit/service.ts (1)
src/repository/statistics.ts (3)
sumKeyTotalCost(770-791)sumUserTotalCost(799-820)sumProviderTotalCost(833-853)
src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx (1)
src/actions/providers.ts (1)
resetProviderTotalUsage(728-748)
src/actions/providers.ts (3)
src/actions/types.ts (1)
ActionResult(31-31)src/lib/auth.ts (1)
getSession(116-128)src/repository/provider.ts (1)
resetProviderTotalCostResetAt(463-474)
src/app/[locale]/settings/providers/_components/forms/provider-form.tsx (4)
scripts/sync-settings-keys.js (1)
t(72-72)src/components/ui/label.tsx (1)
Label(21-21)src/components/ui/input.tsx (1)
Input(21-21)src/lib/utils/validation/index.ts (1)
validateNumericField(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Greptile Review
- GitHub Check: pr-review
- GitHub Check: pr-label
- GitHub Check: pr-description
- GitHub Check: check-codex-status
- GitHub Check: 🌐 API Tests
- GitHub Check: Docker Build Test
- GitHub Check: dev-build-deploy
🔇 Additional comments (38)
messages/zh-TW/settings.json (1)
1164-1164: 翻译文案准确,格式正确新增的总消费限额相关翻译文案与现有风格保持一致:
- 占位符格式
${amount}符合 zh-TW 语言区域的约定- 术语选择恰当:"總消費上限"、"重置總用量" 等表述清晰易懂
- 结构与其他语言区域(en、ja、ru、zh-CN)对齐
Also applies to: 1180-1183, 1584-1587
messages/zh-CN/settings.json (1)
778-778: 翻译内容准确,符合简体中文习惯新增的总消费限额和用量重置相关翻译:
- 占位符格式
{amount}正确(zh-CN 使用此格式,与 zh-TW/ja 的${amount}不同是正常的)- "总消费上限"、"重置总用量" 等表述准确自然
- 与其他语言区域的键名和结构保持一致
Also applies to: 812-815, 186-189
messages/ja/settings.json (1)
1164-1164: 日语翻译准确,表述自然新增的限额相关日语翻译质量良好:
- 占位符格式
${amount}符合日语区域约定- "総消費上限 (USD)"、"総用量をリセット" 等表述自然流畅
- 与其他语言文件结构一致
Also applies to: 1180-1183, 1578-1581
drizzle/meta/_journal.json (1)
320-326: 迁移日志条目格式正确新增的 journal 条目遵循现有模式:
- 索引值 45 正确递增
- 时间戳和版本号格式正确
- 与其他迁移条目结构一致
drizzle/0045_mushy_human_torch.sql (1)
1-2: 数据库迁移结构正确新增的两个字段设计合理:
limit_total_usd使用numeric(10, 2)类型,支持最大 $99,999,999.99,适合 USD 金额存储total_cost_reset_at使用timestamp with time zone,适合记录重置时间点- 两列均可为空,正确表达"未设置限额"和"从未重置"的语义
- 列名采用 snake_case,符合数据库命名规范,与代码层的 camelCase 映射关系清晰
src/lib/validation/schemas.ts (2)
400-405: 验证规则定义正确,与现有模式保持一致。
limit_total_usd字段在CreateProviderSchema中的定义:
- 使用
z.coerce.number()正确处理表单数据类型转换- 范围限制(0 到 10,000,000 美元)合理,且高于其他周期性限额
nullable().optional()确保向后兼容性- 错误消息清晰明确
571-576: 更新模式验证规则正确。
limit_total_usd字段在UpdateProviderSchema中的定义与创建模式保持一致,完全符合部分更新的语义要求。src/repository/_shared/transformers.ts (1)
85-89: 字段转换逻辑正确,与现有模式保持一致。新增字段的转换实现:
limitTotalUsd:使用parseFloat并进行了严格的 null/undefined 检查,防止 NaNtotalCostResetAt:正确地将数据库时间戳转换为 Date 对象或 null这两个字段的处理方式与文件中其他类似字段(如
limitDailyUsd、expiresAt)保持一致。totalCostResetAt返回 null(而不是默认的new Date())是正确的,因为它是一个可选的重置时间戳。src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx (1)
260-284: 总用量重置逻辑实现正确,与现有模式保持一致。
handleResetTotalUsage处理器的实现遵循了组件中现有的最佳实践:
- 使用
useTransition管理异步状态- 完整的错误处理和用户反馈(成功/失败 toast)
- 查询失效和路由刷新确保 UI 数据同步
所有相关翻译键已在全部 5 种语言包(en、ja、ru、zh-CN、zh-TW)中定义,代码符合所有编码指南要求。
src/repository/statistics.ts (1)
822-853: 设计意图已确认,无需调整。新增的
sumProviderTotalCost函数与sumKeyTotalCost和sumUserTotalCost存在的关键差异是有意为之的。注释中明确说明了语义:为防止"达到总限额后过期自动恢复",提供商的总消费限额从resetAt起累计到现在(无自动过期),这与 Key/User 的 365 天时间窗口不同。该设计通过
resetProviderTotalCostResetAt函数支持手动重置,体现了对长期禁用能力的需求。从性能角度看,查询通过单一 SUM 聚合且依赖providerId索引,未产生 N+1 问题。src/drizzle/schema.ts (1)
201-202: 字段定义正确,与现有模式保持一致。新增的
limitTotalUsd和totalCostResetAt字段定义良好:
- 使用
numeric(10, 2)与其他 USD 限额字段保持一致- 时间戳字段使用
withTimezone: true支持时区感知操作- 两个字段都是可空的,确保向后兼容性
无需添加额外索引。虽然
totalCostResetAt被用于记录重置时间并在成本计算中使用,但它不在 WHERE 子句中进行过滤,现有的查询模式(按providerId和deletedAt过滤)无法从针对该字段的索引中获益。src/app/v1/_lib/proxy/provider-selector.ts (2)
597-631: 会话复用路径的限额检查逻辑正确。新增的代码正确地在会话复用路径中强制执行了提供商的成本限制检查和总消费上限检查。这确保了会话复用不会绕过"达到限额即禁用"的语义。
代码逻辑清晰:
- 先检查周期性成本限制(5h/daily/weekly/monthly)
- 再检查总消费上限,并正确传递
resetAt选项用于缓存键生成
962-978: filterByLimits 中的总消费上限检查实现正确。在提供商筛选流程中添加了总消费上限检查,确保在初始选择路径中也能正确过滤掉已达到总限额的提供商。
resetAt参数被正确传递,用于支持手动重置后的缓存键区分。tests/unit/lib/rate-limit/cost-limits.test.ts (1)
205-259: 提供商总消费限制的测试覆盖全面。新增的三个测试用例很好地覆盖了关键场景:
- Redis 缓存未命中时回退到数据库并缓存结果(含 resetAt 的缓存键)
resetAt为空时使用none后缀的缓存键- Redis 缓存命中且超限时正确返回
not allowed测试验证了缓存键格式和数据库回退逻辑,与
RateLimitService.checkTotalCostLimit的实现一致。src/types/provider.ts (2)
73-76: 类型定义正确,注释清晰。新增的
limitTotalUsd和totalCostResetAt字段类型定义正确,注释清楚地说明了其用途:
limitTotalUsd: 总消费上限totalCostResetAt: 用于实现"达到总限额后手动重置用量"的语义
153-153: ProviderDisplay 中仅包含 limitTotalUsd 是合理的。
totalCostResetAt不包含在ProviderDisplay中是合理的设计,因为这是一个内部字段,用于后端计算,不需要在前端显示。前端只需要知道限额值即可。tests/unit/proxy/provider-selector-total-limit.test.ts (1)
87-101: 测试断言全面,覆盖了关键参数验证。测试正确验证了
RateLimitService.checkTotalCostLimit的调用参数:
- 提供商 ID
- 实体类型
"provider"- 限额值
resetAt选项对象这确保了 provider-selector 与 rate-limit service 之间的集成正确性。
Also applies to: 147-161
messages/ru/settings.json (2)
1164-1164: 俄语翻译添加正确,与其他语言文件结构一致。新增的翻译键
rateLimit.summary.total和limitTotal块与现有翻译结构保持一致,占位符格式正确。Also applies to: 1180-1183
1578-1581: 重置用量相关翻译完整。新增的
resetUsageTitle、resetUsageSuccess、resetUsageSuccessDesc和resetUsageFailed翻译与功能需求匹配,支持管理员重置提供商总用量的 UI 操作。src/app/v1/_lib/proxy/rate-limit-guard.ts (2)
49-49: 调用签名更新正确,与 RateLimitService API 保持一致。将第四个参数从直接传递
key改为{ keyHash: key.key }对象形式,与checkTotalCostLimit的新签名一致,支持统一的 options 参数模式(可包含keyHash或resetAt)。
79-79: 用户路径的调用简化合理。移除了用户检查时的
undefined第四参数,因为用户实体不需要keyHash或resetAt选项。这使代码更简洁且语义更清晰。tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx (2)
65-73: setNativeValue 辅助函数实现正确。该函数通过访问原型链上的
value属性描述符来设置输入值,这是在虚拟 DOM 环境中模拟原生输入行为的标准做法。
196-199: 断言验证完整,确保 payload 包含正确的 limit_total_usd 值。测试正确验证了:
editProvider被调用恰好一次- payload 包含
limit_total_usd字段limit_total_usd的值为输入的数字10这确保了 UI 输入能正确传递到后端 action。
messages/en/settings.json (1)
663-666: Provider 总消费限额与重置相关文案整体清晰,注意同步多语言
providers.list.resetUsage*这组文案与“重置总用量”入口的行为匹配,语义明确。sections.rateLimit.summary.total和sections.rateLimit.limitTotal与表单里的 “Total Spend Limit (USD)” 字段保持一致,占位符也和前端实现一致(留空表示不限)。建议确认另外 4 个 locale(ja、ru、zh-CN、zh-TW)下的
settings.json也已经补齐相同的 key,避免 next-intl 运行时缺失。Also applies to: 1274-1275, 1308-1311
drizzle/meta/0045_snapshot.json (1)
1270-1281: providers 表新增 limit_total_usd / total_cost_reset_at 字段设计合理
limit_total_usd使用numeric(10, 2),与现有 key/user 级总额度字段保持一致,适合金额场景。total_cost_reset_at作为“手动重置起点”时间戳,由上层sumProviderTotalCost通过created_at >= resetAt使用,不需要单独索引,现有按provider_id的索引足以支撑聚合查询。这里是快照文件,无需额外改动。
src/actions/providers.ts (5)
41-41: LGTM!导入
resetProviderTotalCostResetAt与新增的resetProviderTotalUsage函数配合使用,符合模块职责分离。
207-207: LGTM!正确将
limitTotalUsd添加到ProviderDisplay返回数据中,与其他限额字段(limit5hUsd、limitDailyUsd 等)保持一致。
357-357: LGTM!
addProvider函数的输入类型正确扩展了limit_total_usd参数,与其他限额参数格式一致。
427-427: LGTM!创建 payload 时正确处理
limit_total_usd,使用?? null确保未定义时传入 null,与其他限额字段处理方式一致。
514-514: LGTM!
editProvider函数的输入类型正确扩展了limit_total_usd参数。src/repository/provider.ts (8)
39-40: LGTM!
limitTotalUsd正确添加到createProvider的数据库插入数据中,与其他限额字段(limit5hUsd、limitDailyUsd等)处理方式一致,将 number 转换为 string 存储。
86-87: LGTM!
createProvider返回值正确包含limitTotalUsd和totalCostResetAt字段,确保新创建的 provider 数据完整。
143-144: LGTM!
findProviderList查询正确包含新字段。
211-212: LGTM!
findAllProviders查询正确包含新字段。
273-274: LGTM!
findProviderById查询正确包含新字段。
356-358: LGTM!
updateProvider正确处理limit_total_usd更新,与其他限额字段处理逻辑一致。
418-419: LGTM!
updateProvider返回值正确包含新字段。
457-474: 实现正确,符合 Repository 模式。
resetProviderTotalCostResetAt函数:
- 正确更新
totalCostResetAt和updatedAt- 使用
isNull(providers.deletedAt)确保只操作未删除的记录- 返回布尔值表示操作是否成功
- 实现模式与
deleteProvider一致根据 coding guidelines,Repository 层正确封装了 Drizzle 查询操作。
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'total spend limit' feature for providers. It adds limit_total_usd and total_cost_reset_at columns to the providers database table, along with corresponding Drizzle ORM migrations and snapshot updates. The feature includes UI enhancements in the settings forms to allow setting and displaying this new limit, and a new action to manually reset a provider's total usage. The rate limiting service (RateLimitService) has been extended to enforce this total spend limit for providers, including caching mechanisms and fallback to database queries. The ProxyProviderResolver now incorporates these total cost checks during provider selection and session reuse. Additionally, the RateLimitService.checkTotalCostLimit method was refactored to accept an options object for keyHash and resetAt parameters, and the undefined argument for user total cost checks was removed for consistency, as noted in the review comment.
| user.limitTotalUsd ?? null, | ||
| undefined | ||
| user.limitTotalUsd ?? null | ||
| ); |
| id={isEdit ? "edit-limit-total" : "limit-total"} | ||
| type="number" | ||
| value={limitTotalUsd?.toString() ?? ""} | ||
| onChange={(e) => setLimitTotalUsd(validateNumericField(e.target.value))} |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] limit_total_usd input truncates decimals
Why this is a problem: In src/app/[locale]/settings/providers/_components/forms/provider-form.tsx:1169 the new Total Spend Limit input uses:
onChange={(e) => setLimitTotalUsd(validateNumericField(e.target.value))}
But validateNumericField is integer-only (parseInt), so values like "10.50" become "10" and "0.5" becomes null. This silently changes the configured limit and can lead to incorrect provider total-limit enforcement.
Suggested fix:
// Use a decimal-aware validator for USD limits (including limit_total_usd)
onChange={(e) => setLimitTotalUsd(validatePositiveDecimalField(e.target.value))}export function validatePositiveDecimalField(value: string): number | null {
if (!value.trim()) return null;
const num = Number.parseFloat(value);
if (!Number.isFinite(num) || num <= 0) return null;
return Math.round(num * 100) / 100;
}There was a problem hiding this comment.
Code Review Summary
This PR is XL and spans DB migrations, backend enforcement, proxy selection logic, admin UI, and unit tests. The main blocker is a high-confidence UI validation bug that silently truncates decimals for limit_total_usd, which can misconfigure total spend limits.
PR Size: XL
- Lines changed: 3078
- Files changed: 23
- Split suggestions: split into (1) Drizzle migration/snapshots, (2) rate-limit + provider-selector enforcement, (3) settings UI + i18n, (4) tests.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 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 |
Critical Issues (Must Fix)
src/app/[locale]/settings/providers/_components/forms/provider-form.tsx:1169[HIGH] [LOGIC-BUG]validateNumericFieldusesparseInt, truncating decimal USD limits forlimit_total_usd.
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.
- Identified PR
#535(fix: provider total limit editable + enforce + reset) and applied labelsize/XL. - Posted 1 inline review comment on
src/app/[locale]/settings/providers/_components/forms/provider-form.tsx:1169flagging a [HIGH] [LOGIC-BUG]:validateNumericFieldusesparseInt, silently truncating decimal values forlimit_total_usd. - Submitted the required PR review summary (includes XL split suggestions and the issue breakdown table).
|
已按评审意见修复两点:
同时补充/更新单测:
对应提交:9c23db50 |
🧪 测试结果
总体结果: ✅ 所有测试通过 |
|
Greptile encountered an error while reviewing this PR. Please reach out to support@greptile.com for assistance. |
修复供应商总限额(limit_total_usd)在前端不可见/不可编辑的问题,并补齐端到端生效路径。
关联 PR (Related PRs)
本 PR 是限额管理系统演进的一部分:
本 PR 将 Provider 层总限额功能补全,使其与 Key/User 层限额体系对齐,形成完整的三层限额控制(Provider -> User -> Key)。
技术细节
数据库变更 (Migration 0045)
核心文件
src/app/v1/_lib/proxy/provider-selector.ts- 供应商选择时校验总限额 + 会话复用路径强制重校验src/lib/rate-limit/service.ts- 新增 Provider 总限额检查支持(checkCostLimits with total period)src/app/[locale]/settings/providers/_components/provider-form.tsx- 表单新增总限额字段src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx- 列表项新增"重置总用量"操作测试覆盖
tests/unit/lib/rate-limit/cost-limits.test.ts- 补齐 total 限额检查、缓存 key 命名、resetAt 语义单测tests/unit/proxy/provider-selector-total-limit.test.ts- 供应商选择器总限额场景(初选 + 复用)tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx- 表单 UI 总限额字段渲染与验证Breaking Changes
✅ 无破坏性变更
Description enhanced by Claude AI