feat(users): sync user limits to keys#1112
Conversation
📝 WalkthroughWalkthrough新增“同步用户配置到 Keys”后端逻辑与构建器、在仪表板的用户创建/编辑/批量编辑中加入对应 UI 控件与流程，重组设置页面的账单纠正分组，并添加多语言文案与单元测试覆盖。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a "Sync to Keys" feature, allowing administrators to synchronize user-level configurations—including rate limits and provider groups—to all associated API keys during individual or batch edits. The implementation includes new server actions, a utility for distributing limits across multiple keys, and UI enhancements in the user management and system settings dashboards, such as a new "Billing and Rate Correction" section. Review feedback highlights several performance and maintainability improvements: addressing an N+1 update pattern in the batch synchronization loop, moving cache invalidations to fire-and-forget tasks to reduce latency, and replacing hardcoded strings with localized messages. Additionally, it is recommended to include the missing limit5hResetMode field in the synchronization logic to ensure full configuration parity.
| for (const [index, keyRow] of userKeys.entries()) { | ||
| const config = configs[index]; | ||
| await tx | ||
| .update(keysTable) | ||
| .set({ | ||
| updatedAt: new Date(), | ||
| limit5hUsd: config.limit5hUsd === null ? null : config.limit5hUsd.toString(), | ||
| limitDailyUsd: config.limitDailyUsd === null ? null : config.limitDailyUsd.toString(), | ||
| dailyResetMode: config.dailyResetMode, | ||
| dailyResetTime: config.dailyResetTime, | ||
| limitWeeklyUsd: | ||
| config.limitWeeklyUsd === null ? null : config.limitWeeklyUsd.toString(), | ||
| limitMonthlyUsd: | ||
| config.limitMonthlyUsd === null ? null : config.limitMonthlyUsd.toString(), | ||
| limitTotalUsd: config.limitTotalUsd === null ? null : config.limitTotalUsd.toString(), | ||
| limitConcurrentSessions: config.limitConcurrentSessions, | ||
| providerGroup: config.providerGroup, | ||
| }) | ||
| .where(and(eq(keysTable.id, keyRow.id), isNull(keysTable.deletedAt))); | ||
| } |
There was a problem hiding this comment.
Updating keys one by one in a loop inside a transaction results in an N+1 update pattern. With a batch size of up to 500 users and potentially multiple keys per user, this can lead to significant performance degradation and extended lock times on the keys table. Consider using a more efficient batch update strategy, such as a single SQL statement with a CASE expression or a temporary table join, if supported by the database and ORM.
| const missingIds = requestedIds.filter((id) => !existingSet.has(id)); | ||
| if (missingIds.length > 0) { | ||
| throw new BatchUpdateError( | ||
| `部分用户不存在: ${missingIds.join(", ")}`, |
There was a problem hiding this comment.
The error message is hardcoded in Chinese, which is inconsistent with the internationalization strategy used elsewhere in the project. Please use the tError function to retrieve a localized message from the errors namespace.
| await Promise.all([ | ||
| ...requestedIds.map((userId) => | ||
| invalidateCachedUser(userId).catch((error) => | ||
| warnUserCacheInvalidationFailure("batch user-key sync", userId, error) | ||
| ) | ||
| ), | ||
| ...keyStringsForCache.map((key) => | ||
| invalidateCachedKey(key).catch((error) => | ||
| warnKeyCacheInvalidationFailure("batch user-key sync", key, error) | ||
| ) | ||
| ), | ||
| ]); |
There was a problem hiding this comment.
Executing a large number of concurrent cache invalidations using Promise.all can overwhelm the Redis connection pool or the event loop. In performance-sensitive paths, non-critical I/O operations like Redis cache invalidation should be executed as fire-and-forget tasks to avoid blocking the main logic. Consider triggering these invalidations without awaiting their completion.
References
- In performance-sensitive code paths, non-critical I/O operations (e.g., releasing a session in Redis) should be executed as fire-and-forget tasks to avoid blocking the main logic.
| logger.error("批量同步用户配置到 Key 失败:", error); | ||
| const message = error instanceof Error ? error.message : "批量同步用户配置到 Key 失败"; |
There was a problem hiding this comment.
Log messages and fallback error messages are hardcoded in Chinese. For consistency and better maintainability, please use English for logs and the tError function for user-facing error messages.
| const { code, params } = await import("@/lib/utils/error-messages").then((m) => | ||
| m.zodErrorToCode(issue.code, { | ||
| minimum: "minimum" in issue ? issue.minimum : undefined, | ||
| maximum: "maximum" in issue ? issue.maximum : undefined, | ||
| type: "expected" in issue ? issue.expected : undefined, | ||
| received: "received" in issue ? issue.received : undefined, | ||
| validation: "validation" in issue ? issue.validation : undefined, | ||
| path: issue.path, | ||
| message: "message" in issue ? issue.message : undefined, | ||
| params: "params" in issue ? issue.params : undefined, | ||
| }) |
There was a problem hiding this comment.
This dynamic import inside a server action can add unnecessary latency to every request. It is recommended to move this import to the top level of the file to ensure it is loaded once during initialization.
| export interface UserKeySyncSource { | ||
| dailyQuota?: number | string | null; | ||
| limit5hUsd?: number | string | null; | ||
| limitWeeklyUsd?: number | string | null; | ||
| limitMonthlyUsd?: number | string | null; | ||
| limitTotalUsd?: number | string | null; | ||
| limitConcurrentSessions?: number | string | null; | ||
| providerGroup?: string | null; | ||
| dailyResetMode?: "fixed" | "rolling" | null; | ||
| dailyResetTime?: string | null; | ||
| } | ||
|
|
||
| export interface SyncedKeyConfig { | ||
| limit5hUsd: number | null; | ||
| limitDailyUsd: number | null; | ||
| limitWeeklyUsd: number | null; | ||
| limitMonthlyUsd: number | null; | ||
| limitTotalUsd: number | null; | ||
| limitConcurrentSessions: number; | ||
| providerGroup: string; | ||
| dailyResetMode: "fixed" | "rolling"; | ||
| dailyResetTime: string; | ||
| } |
There was a problem hiding this comment.
The limit5hResetMode field is missing from the synchronization interfaces. This field should be included to ensure that the 5-hour limit reset behavior (fixed vs. rolling) is correctly synced from users to their keys, maintaining consistency across the limit configuration.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| throw new BatchUpdateError( | ||
| `部分用户不存在: ${missingIds.join(", ")}`, | ||
| ERROR_CODES.NOT_FOUND | ||
| ); |
There was a problem hiding this comment.
BatchUpdateError's message is returned verbatim as result.error in the catch block and surfaced via toast.error(t("toast.syncFailed", { error: errorMessage })). Non-Chinese users will see "部分用户不存在: 3, 7" in the toast. The identical pattern already exists in batchUpdateUsers (line 1166, pre-existing), but this PR adds a second occurrence. Use a translation key or the tError("USER_NOT_FOUND") pattern used elsewhere in the same file.
| throw new BatchUpdateError( | |
| `部分用户不存在: ${missingIds.join(", ")}`, | |
| ERROR_CODES.NOT_FOUND | |
| ); | |
| throw new BatchUpdateError( | |
| tError("USER_NOT_FOUND"), | |
| ERROR_CODES.NOT_FOUND | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 1376-1379
Comment:
**Hardcoded Chinese user-facing error string**
`BatchUpdateError`'s message is returned verbatim as `result.error` in the catch block and surfaced via `toast.error(t("toast.syncFailed", { error: errorMessage }))`. Non-Chinese users will see `"部分用户不存在: 3, 7"` in the toast. The identical pattern already exists in `batchUpdateUsers` (line 1166, pre-existing), but this PR adds a second occurrence. Use a translation key or the `tError("USER_NOT_FOUND")` pattern used elsewhere in the same file.
```suggestion
throw new BatchUpdateError(
tError("USER_NOT_FOUND"),
ERROR_CODES.NOT_FOUND
);
```
How can I resolve this? If you propose a fix, please make it concise.| logger.error("批量同步用户配置到 Key 失败:", error); | ||
| const message = error instanceof Error ? error.message : "批量同步用户配置到 Key 失败"; |
There was a problem hiding this comment.
The fallback string "批量同步用户配置到 Key 失败" can be returned to the caller as result.error when the caught error has no message. Compare to syncUserConfigToKeys (same file) which correctly calls tError("UPDATE_USER_FAILED") for its generic fallback.
| logger.error("批量同步用户配置到 Key 失败:", error); | |
| const message = error instanceof Error ? error.message : "批量同步用户配置到 Key 失败"; | |
| logger.error("Failed to batch-sync user config to keys:", error); | |
| const tError = await getTranslations("errors"); | |
| const message = error instanceof Error ? error.message : tError("UPDATE_USER_FAILED"); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 1484-1485
Comment:
**Hardcoded Chinese fallback error message**
The fallback string `"批量同步用户配置到 Key 失败"` can be returned to the caller as `result.error` when the caught error has no message. Compare to `syncUserConfigToKeys` (same file) which correctly calls `tError("UPDATE_USER_FAILED")` for its generic fallback.
```suggestion
logger.error("Failed to batch-sync user config to keys:", error);
const tError = await getTranslations("errors");
const message = error instanceof Error ? error.message : tError("UPDATE_USER_FAILED");
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| const handleSyncKeys = async () => { | ||
| setIsSyncingKeys(true); | ||
| try { | ||
| const data = form.values || defaultValues; | ||
| const res = await syncUserConfigToKeys(user.id, { | ||
| name: data.name, | ||
| note: data.note, | ||
| tags: data.tags, | ||
| expiresAt: data.expiresAt ?? null, | ||
| providerGroup: normalizeProviderGroup(data.providerGroup), | ||
| rpm: data.rpm, | ||
| limit5hUsd: data.limit5hUsd, | ||
| dailyQuota: data.dailyQuota, | ||
| limitWeeklyUsd: data.limitWeeklyUsd, | ||
| limitMonthlyUsd: data.limitMonthlyUsd, | ||
| limitTotalUsd: data.limitTotalUsd, | ||
| limitConcurrentSessions: data.limitConcurrentSessions, | ||
| dailyResetMode: data.dailyResetMode, | ||
| dailyResetTime: data.dailyResetTime, | ||
| allowedClients: data.allowedClients, | ||
| blockedClients: data.blockedClients, | ||
| allowedModels: data.allowedModels, | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| toast.error(res.error || t("editDialog.syncKeys.error")); | ||
| return; | ||
| } | ||
|
|
||
| toast.success(t("editDialog.syncKeys.success", { count: res.data.keyCount })); | ||
| onSuccess?.(); | ||
| queryClient.invalidateQueries({ queryKey: ["users"] }); | ||
| queryClient.invalidateQueries({ queryKey: ["userKeyGroups"] }); | ||
| queryClient.invalidateQueries({ queryKey: ["userTags"] }); | ||
| router.refresh(); |
There was a problem hiding this comment.
syncUserConfigToKeys receives the full current form state — including name, note, tags, expiresAt, allowedClients, etc. — and the server action writes all those fields to the user record via buildUserDbUpdates before syncing limits to keys. An admin who has unsaved edits and clicks "Sync to Keys" will unknowingly persist name/note/tag changes too. The button label "Sync to Keys" gives no indication of this side effect.
Consider either (a) only passing the sync-relevant fields (providerGroup, dailyQuota, limit*, dailyReset*, limitConcurrentSessions) to the action, or (b) updating the button tooltip/label to communicate that form changes are saved first.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/_components/user/edit-user-dialog.tsx
Line: 307-342
Comment:
**"Sync to Keys" implicitly saves all unsaved form changes**
`syncUserConfigToKeys` receives the full current form state — including `name`, `note`, `tags`, `expiresAt`, `allowedClients`, etc. — and the server action writes all those fields to the user record via `buildUserDbUpdates` before syncing limits to keys. An admin who has unsaved edits and clicks "Sync to Keys" will unknowingly persist name/note/tag changes too. The button label "Sync to Keys" gives no indication of this side effect.
Consider either (a) only passing the sync-relevant fields (`providerGroup`, `dailyQuota`, `limit*`, `dailyReset*`, `limitConcurrentSessions`) to the action, or (b) updating the button tooltip/label to communicate that form changes are saved first.
How can I resolve this? If you propose a fix, please make it concise.| for (const [index, keyRow] of userKeys.entries()) { | ||
| const config = configs[index]; | ||
| await tx | ||
| .update(keysTable) | ||
| .set({ | ||
| updatedAt: new Date(), | ||
| limit5hUsd: config.limit5hUsd === null ? null : config.limit5hUsd.toString(), | ||
| limitDailyUsd: config.limitDailyUsd === null ? null : config.limitDailyUsd.toString(), | ||
| dailyResetMode: config.dailyResetMode, | ||
| dailyResetTime: config.dailyResetTime, | ||
| limitWeeklyUsd: | ||
| config.limitWeeklyUsd === null ? null : config.limitWeeklyUsd.toString(), | ||
| limitMonthlyUsd: | ||
| config.limitMonthlyUsd === null ? null : config.limitMonthlyUsd.toString(), | ||
| limitTotalUsd: config.limitTotalUsd === null ? null : config.limitTotalUsd.toString(), | ||
| limitConcurrentSessions: config.limitConcurrentSessions, | ||
| providerGroup: config.providerGroup, | ||
| }) | ||
| .where(and(eq(keysTable.id, keyRow.id), isNull(keysTable.deletedAt))); | ||
| } | ||
|
|
||
| userResults.push({ | ||
| updatedUserId: userRow.id, | ||
| updatedKeyIds: userKeys.map((keyRow) => keyRow.id), | ||
| keyCount: userKeys.length, | ||
| summary, |
There was a problem hiding this comment.
Each key is updated with a separate await tx.update(keysTable).set(config).where(eq(keysTable.id, keyRow.id)) call. For a batch of 500 users with even a modest average of 3 keys each, this results in ~1,500 round-trips to the DB within one transaction, which will be slow and may hit connection or statement timeouts. At minimum, consider adding a comment about this known limitation and a practical upper bound on users-per-batch that keeps latency acceptable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 1418-1443
Comment:
**N+1 key update queries inside the transaction**
Each key is updated with a separate `await tx.update(keysTable).set(config).where(eq(keysTable.id, keyRow.id))` call. For a batch of 500 users with even a modest average of 3 keys each, this results in ~1,500 round-trips to the DB within one transaction, which will be slow and may hit connection or statement timeouts. At minimum, consider adding a comment about this known limitation and a practical upper bound on users-per-batch that keeps latency acceptable.
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 588a923aea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const newKey = await createKey({ | ||
| user_id: newUser.id, | ||
| name: "default", | ||
| key: generatedKey, | ||
| is_enabled: true, | ||
| expires_at: undefined, |
There was a problem hiding this comment.
addUser now creates the user first and the default key second, but this path is not wrapped in a transaction and does not roll back the user if key creation fails. In any partial-failure case (e.g. DB error during createKey), the action returns failure while leaving a persisted user without the expected default key, which can produce orphaned users and duplicate users on retry.
Useful? React with 👍 / 👎.
| limit_5h_usd: defaultKeyConfig.limit5hUsd, | ||
| limit_daily_usd: defaultKeyConfig.limitDailyUsd, | ||
| daily_reset_mode: defaultKeyConfig.dailyResetMode, | ||
| daily_reset_time: defaultKeyConfig.dailyResetTime, | ||
| limit_weekly_usd: defaultKeyConfig.limitWeeklyUsd, |
There was a problem hiding this comment.
This new default-key creation payload copies quota fields from the user but omits limit_5h_reset_mode. Because createKey defaults that field to "rolling", creating a user with limit5hResetMode: "fixed" will still produce a default key with rolling 5h reset behavior, so the key no longer matches the user's configured limit reset policy.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
tests/unit/user-dialogs.test.tsx (1)
350-350: 建议通过messages常量引用按钮文本，避免硬编码。文件中其他断言（如第 366、367 行）都通过
messages.dashboard.userManagement.editDialog.reset5h.button这种方式引用 i18n 文案，而此处直接硬编码了"Sync to Keys"。如果未来英文文案调整（例如改成 "Sync to keys"），测试会失败但代码本身没问题。建议保持一致性。♻️ 推荐写法
- expect(buttonTexts).toContain("Sync to Keys"); + expect(buttonTexts).toContain( + messages.dashboard.userManagement.editDialog.syncKeys.button + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/user-dialogs.test.tsx` at line 350, Replace the hardcoded button label check expect(buttonTexts).toContain("Sync to Keys"); with the i18n messages constant used elsewhere—use messages.dashboard.userManagement.editDialog.reset5h.button (the same symbol used in other assertions) so the test references the localized string instead of a literal.src/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsx (1)
469-542: 建议合并showProviderGroup的条件分支以减少重复判断。当前 4 个分支都以
showProviderGroup &&开头，可以将整个三元链统一包裹一层showProviderGroup && (...)，提升可读性。♻️ 推荐重构
- {showProviderGroup && isAdmin ? ( - <ProviderGroupSelect - value={keyData.providerGroup || PROVIDER_GROUP.DEFAULT} - onChange={(val) => onChange("providerGroup", val)} - disabled={false} - translations={{ - label: translations.fields.providerGroup.label, - placeholder: translations.fields.providerGroup.placeholder, - }} - /> - ) : showProviderGroup && userGroups.length > 0 ? ( - <div className="space-y-2"> - {/* ... */} - </div> - ) : showProviderGroup && keyGroupOptions.length > 0 ? ( - <div className="space-y-2"> - {/* ... */} - </div> - ) : showProviderGroup ? ( - <div className="text-sm text-muted-foreground"> - {translations.fields.providerGroup.noGroupHint || "您没有分组限制，可以访问所有供应商"} - </div> - ) : null} + {showProviderGroup && + (isAdmin ? ( + <ProviderGroupSelect ... /> + ) : userGroups.length > 0 ? ( + <div className="space-y-2">{/* ... */}</div> + ) : keyGroupOptions.length > 0 ? ( + <div className="space-y-2">{/* ... */}</div> + ) : ( + <div className="text-sm text-muted-foreground"> + {translations.fields.providerGroup.noGroupHint || "您没有分组限制，可以访问所有供应商"} + </div> + ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/key-edit-section.tsx around lines 469 - 542, The JSX repeats "showProviderGroup &&" across several ternary branches; refactor by extracting a single outer guard: wrap the entire multi-branch expression with showProviderGroup && ( ... ) and inside keep the existing nested ternaries/JSX for the isAdmin branch (ProviderGroupSelect), the userGroups branch (read-only Badge list when keyData.id > 0 and TagInputField when creating), the keyGroupOptions branch, and the fallback hint; ensure you reference and preserve logic around isAdmin, ProviderGroupSelect, TagInputField, keyGroupOptions, userGroups and keyData.id so behavior and props (onChange/handleUserProviderGroupChange/translations) remain unchanged.src/actions/users.ts (4)
2253-2261: 兜底默认值result ?? {...}中buildSyncedKeyConfigs({}, 0).summary在每次回退时重新构造,可提取为常量事务正常完成时
result必然被赋值,该兜底分支只在防御性场景中触发(理论上不可达)。但更优写法是把空 summary 抽成const EMPTY_SYNC_SUMMARY = buildSyncedKeyConfigs({}, 0).summary(模块级常量)避免每次返回时重新计算,也让"空场景下的 summary 形状"显式化。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 2253 - 2261, The fallback object reconstructs buildSyncedKeyConfigs({}, 0).summary on every call; extract that empty summary into a module-level constant (e.g. const EMPTY_SYNC_SUMMARY = buildSyncedKeyConfigs({}, 0).summary) and replace buildSyncedKeyConfigs({}, 0).summary in the fallback of the return with EMPTY_SYNC_SUMMARY; update usages in this module to import/use that constant so the "empty" summary shape is computed once and explicit (reference: buildSyncedKeyConfigs and the fallback result ?? { ... summary: ... }).
1373-1380: 用户存在性校验放在 UPDATE 之后,行为符合预期但有一个细微点需确认Line 1349-1354 先对
requestedIds范围内"未删除"用户执行 UPDATE;Line 1356-1371 再 SELECT 现存用户;Line 1373-1380 抛BatchUpdateError(NOT_FOUND)。由于在事务内,抛异常会回滚 UPDATE,语义正确。但与
batchUpdateUsers的对称实现(Line 1156-1201:先 SELECT 存在性,再 UPDATE,再断言行数)不同,本实现 UPDATE 时不报错,而是事后查存在性。两者最终都正确,但建议保持一致风格(先校验存在性、再 UPDATE),以便未来在 UPDATE 加RETURNING计数差异时不会出现两套断言路径。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 1373 - 1380, The current routine updates rows first then SELECTs to detect missing IDs (using requestedIds, existingSet, missingIds, and throwing BatchUpdateError with ERROR_CODES.NOT_FOUND); change it to match batchUpdateUsers' style: first SELECT the non-deleted users for requestedIds and throw BatchUpdateError if any are missing, then perform the UPDATE (and optionally use RETURNING or check affected row count) so existence checks occur before mutation and both paths share the same assertion pattern as batchUpdateUsers.
2092-2230:syncUserConfigToKeys与batchSyncUserConfigToKeys的 Key 更新内循环高度重复Line 2211-2230 的 Key UPDATE 块与 Line 1418-1437 的批量版本字段映射完全相同。可抽取一个
applySyncedKeyConfig(tx, keyId, config)helper 复用,既减少重复,又能在以后字段增减时只改一处:♻️ 提炼共享 helper
+async function applySyncedKeyConfig( + tx: Parameters<Parameters<typeof db.transaction>[0]>[0], + keyId: number, + config: SyncedKeyConfig +): Promise<void> { + await tx + .update(keysTable) + .set({ + updatedAt: new Date(), + limit5hUsd: config.limit5hUsd === null ? null : config.limit5hUsd.toString(), + limitDailyUsd: config.limitDailyUsd === null ? null : config.limitDailyUsd.toString(), + dailyResetMode: config.dailyResetMode, + dailyResetTime: config.dailyResetTime, + limitWeeklyUsd: + config.limitWeeklyUsd === null ? null : config.limitWeeklyUsd.toString(), + limitMonthlyUsd: + config.limitMonthlyUsd === null ? null : config.limitMonthlyUsd.toString(), + limitTotalUsd: config.limitTotalUsd === null ? null : config.limitTotalUsd.toString(), + limitConcurrentSessions: config.limitConcurrentSessions, + providerGroup: config.providerGroup, + }) + .where(and(eq(keysTable.id, keyId), isNull(keysTable.deletedAt))); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 2092 - 2230, The key-update block in syncUserConfigToKeys duplicates the same field mapping used in batchSyncUserConfigToKeys; create a helper function (e.g., applySyncedKeyConfig) that accepts the transaction (tx), keyId (or keyRow.id) and a config object and performs the update against keysTable with the exact fields: updatedAt, limit5hUsd, limitDailyUsd, dailyResetMode, dailyResetTime, limitWeeklyUsd, limitMonthlyUsd, limitTotalUsd, limitConcurrentSessions, providerGroup (converting numeric/null limits to strings as currently done); replace the inline tx.update(...) loop in syncUserConfigToKeys (and the loop in batchSyncUserConfigToKeys) to call this helper to remove duplication and keep mapping logic in one place.
284-303:EditableUserData类型与buildUserDbUpdates处理范围一致,但与UpdateUserSchema字段集存在差异类型上不包含
limit5hResetMode,而syncUserConfigToKeys在 Line 2116 用UpdateUserSchema.safeParse(data)校验——若调用方通过as any/类型断言传入limit5hResetMode,Zod 会保留它(默认 strip 未知键,但已知键会保留),而buildUserDbUpdates(Line 319-368)并未处理该字段,会被静默丢弃。考虑到:
- TypeScript 类型已限制了显式调用方的入参;
limit5hResetMode改变时batchUpdateUsers需要清理 Redis 5h 缓存(Line 1203-1224),而本路径没有该清理逻辑;建议在
buildUserDbUpdates顶部加注释明确"本 helper 仅处理同步到 Key 的字段子集,limit5hResetMode等仅用户级字段不在此处理",或通过pick()派生一个更窄的 schema 用于syncUserConfigToKeys,避免后续维护者混淆字段范围。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 284 - 303, The helper buildUserDbUpdates only handles a subset of user fields synced to keys but UpdateUserSchema (used in syncUserConfigToKeys) can accept extra fields like limit5hResetMode which buildUserDbUpdates currently ignores and thus silently drops (and also misses the Redis 5h cache cleanup logic in batchUpdateUsers). Either: add a clear comment at the top of buildUserDbUpdates stating it only processes the subset of fields that are synced to keys and that user-only fields (e.g., limit5hResetMode) are intentionally not handled here; or better, derive a narrower Zod/TypeScript schema via pick() for syncUserConfigToKeys that only permits the keys buildUserDbUpdates expects (so unknown fields are rejected), and if you intend to support limit5hResetMode here, add explicit handling in buildUserDbUpdates plus the corresponding Redis cache cleanup logic used by batchUpdateUsers. Ensure references: EditableUserData, buildUserDbUpdates, syncUserConfigToKeys, UpdateUserSchema, batchUpdateUsers are updated accordingly.src/lib/users/user-key-sync.ts (2)
183-185:buildFirstSyncedKeyConfig在 keyCount=1 时一定返回非空,但类型上未被静态保证
buildSyncedKeyConfigs(source, 1).configs[0]在硬编码keyCount=1时一定有元素;但若未来keyCount被参数化或Math.max(0, Math.floor(keyCount))边界条件被重构,configs[0]可能为undefined,而函数签名仍声明返回SyncedKeyConfig(非可选),会让调用方在 strict 模式下静默拿到undefined。可加一行防御:♻️ 防御性写法
export function buildFirstSyncedKeyConfig(source: UserKeySyncSource): SyncedKeyConfig { - return buildSyncedKeyConfigs(source, 1).configs[0]; + const { configs } = buildSyncedKeyConfigs(source, 1); + if (!configs[0]) { + throw new Error("buildFirstSyncedKeyConfig: failed to build first config"); + } + return configs[0]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/users/user-key-sync.ts` around lines 183 - 185, buildFirstSyncedKeyConfig currently returns buildSyncedKeyConfigs(source, 1).configs[0] which assumes configs[0] exists; to make this safe change buildFirstSyncedKeyConfig to defensively handle missing element by checking the result of buildSyncedKeyConfigs(source, 1).configs and throwing a clear error or asserting when configs[0] is undefined, ensuring the function always either returns a valid SyncedKeyConfig or fails fast (reference: buildFirstSyncedKeyConfig, buildSyncedKeyConfigs, SyncedKeyConfig, configs).
70-100: 确认:小额配额下的"前 N 个 Key 各得 1 分"分配是有意为之当
totalCents < keyCount时,实现把前totalCents个 Key 各分 1 分,其余 Key 设为null(无限制)。例如用户 5h USD = 0.02、3 个 Key,会得到[0.01, 0.01, null],这意味着排在后面的 Key 在该字段上无配额上限。这与distributeConcurrentSessions的行为一致(余下的 Key 得 0,即明确为零)。两个函数对"无法平均分摊"语义不同(nullvs0),建议在文件顶部加一行 JSDoc 说明:limit*Usd字段下"分配不到的 Key 视为不限制",limitConcurrentSessions字段下"分配不到的 Key 视为零"——避免后续维护者误以为是 bug。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/users/user-key-sync.ts` around lines 70 - 100, The file lacks a top-level JSDoc clarifying divergent semantics between monetary and session distribution: update the header of src/lib/users/user-key-sync.ts to document that distributeAmount (used for limit*Usd fields) intentionally gives the first N keys 0.01 when totalCents < keyCount and sets the remaining keys to null to mean "no limit", whereas distributeConcurrentSessions (and limitConcurrentSessions) treats remaining keys as 0 (explicit zero capacity); mention the exact fields (limit*Usd and limitConcurrentSessions), the functions distributeAmount and distributeConcurrentSessions, and that this is intentional to avoid future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/ja/dashboard.json`:
- Around line 1684-1685: The Japanese locale file contains English text for the
keys "syncKeys" and "syncKeysDescription" (and additional English strings around
the same area at the other noted entries), so replace those English values with
proper Japanese translations to keep UI language consistent; locate the keys
"syncKeys", "syncKeysDescription" and the other affected keys in
messages/ja/dashboard.json (the entries reported at the nearby blocks) and
update their string values to Japanese equivalents that convey the meaning
("同期するキー" etc.), ensuring placeholders like {users} remain intact and
punctuation/escaping matches the JSON style used elsewhere.
In `@messages/ru/settings/config.json`:
- Around line 16-19: Translate the untranslated English strings under
billingCorrection in messages/ru/settings/config.json: replace
billingCorrection.title and billingCorrection.description with proper Russian
translations (matching style used in ja/zh-CN/zh-TW entries) so the settings
page header/button shown by src/app/[locale]/settings/config/page.tsx is
localized; update the JSON values for the keys "billingCorrection.title" and
"billingCorrection.description" accordingly and ensure valid JSON string
quoting/escaping.
In `@src/actions/users.ts`:
- Around line 1382-1437: The current batchSyncUserConfigToKeys logic issues a
separate tx.update for each key (loop around keysTable updates), causing many
serial round-trips and long transactions; replace the per-key updates inside
batchSyncUserConfigToKeys by building a single batched UPDATE using Drizzle's
sql template with CASE WHEN keyed by keysTable.id (similar to the pattern in
src/repository/provider.ts lines ~954-964 and
src/repository/message-write-buffer.ts lines ~116-160): collect all key ids and
for each updatable column (limit5hUsd, limitDailyUsd, limitWeeklyUsd,
limitMonthlyUsd, limitTotalUsd, limitConcurrentSessions, providerGroup,
dailyResetMode, dailyResetTime, updatedAt) build CASE expressions mapping id ->
value (use NULL where appropriate), then execute one tx.sql`UPDATE ${keysTable}
SET ... WHERE ${inArray(keysTable.id, ids)} AND ${isNull(keysTable.deletedAt)}`
inside the transaction to replace the looped tx.update calls.
In
`@src/app/`[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsx:
- Around line 441-443: The confirmation copy underestimates affected keys when
syncUsersToKeys is true but no individual keys are selected: change the
keysCount computation and the copy-generation logic so that if
willSyncUsersToKeys && !willUpdateKeys you do not treat keysCount as 0; instead
set a sentinel (e.g., undefined or a boolean like syncingAllKeys) and update the
UI text path to either omit a numeric key count or explicitly say "同步这些用户下的全部未删除
key"/"all non-deleted keys of these users"; update the variables/expressions
around willSyncUsersToKeys, willUpdateKeys, keysCount, usersCount and the
confirmation-copy rendering in the batch-edit-dialog component (the block that
reads pendingUpdate.keyIds / pendingUpdate.userIds and the message construction
used for the dialog) and make the identical change in the other confirmation
block noted (the similar logic around the later block referenced).
In `@src/app/`[locale]/dashboard/_components/user/create-user-dialog.tsx:
- Around line 175-185: The code builds the default key config from the form
draft (data.user) which can differ from the persisted user returned by the
server; change the construction of firstKeyConfig (the call to
buildFirstSyncedKeyConfig) and any subsequent use of limit5hResetMode to derive
values from the server response object userRes.data.user instead of data.user so
the first API key reflects server-side defaults (also update the analogous logic
referenced around lines 194-204 to use userRes.data.user).
In `@src/app/`[locale]/dashboard/_components/user/edit-user-dialog.tsx:
- Around line 312-329: The syncUserConfigToKeys call is missing the
limit5hResetMode field, so updates to that setting aren't persisted or
propagated when using the "Sync to Keys" path; update the argument object passed
to syncUserConfigToKeys (the call in edit-user-dialog.tsx that includes name,
note, tags, expiresAt, providerGroup, rpm, limit5hUsd, dailyQuota,
limitWeeklyUsd, limitMonthlyUsd, limitTotalUsd, limitConcurrentSessions,
dailyResetMode, dailyResetTime, allowedClients, blockedClients, allowedModels)
to also include limit5hResetMode: data.limit5hResetMode (ensuring the same key
name as used by the regular save path).
---
Nitpick comments:
In `@src/actions/users.ts`:
- Around line 2253-2261: The fallback object reconstructs
buildSyncedKeyConfigs({}, 0).summary on every call; extract that empty summary
into a module-level constant (e.g. const EMPTY_SYNC_SUMMARY =
buildSyncedKeyConfigs({}, 0).summary) and replace buildSyncedKeyConfigs({},
0).summary in the fallback of the return with EMPTY_SYNC_SUMMARY; update usages
in this module to import/use that constant so the "empty" summary shape is
computed once and explicit (reference: buildSyncedKeyConfigs and the fallback
result ?? { ... summary: ... }).
- Around line 1373-1380: The current routine updates rows first then SELECTs to
detect missing IDs (using requestedIds, existingSet, missingIds, and throwing
BatchUpdateError with ERROR_CODES.NOT_FOUND); change it to match
batchUpdateUsers' style: first SELECT the non-deleted users for requestedIds and
throw BatchUpdateError if any are missing, then perform the UPDATE (and
optionally use RETURNING or check affected row count) so existence checks occur
before mutation and both paths share the same assertion pattern as
batchUpdateUsers.
- Around line 2092-2230: The key-update block in syncUserConfigToKeys duplicates
the same field mapping used in batchSyncUserConfigToKeys; create a helper
function (e.g., applySyncedKeyConfig) that accepts the transaction (tx), keyId
(or keyRow.id) and a config object and performs the update against keysTable
with the exact fields: updatedAt, limit5hUsd, limitDailyUsd, dailyResetMode,
dailyResetTime, limitWeeklyUsd, limitMonthlyUsd, limitTotalUsd,
limitConcurrentSessions, providerGroup (converting numeric/null limits to
strings as currently done); replace the inline tx.update(...) loop in
syncUserConfigToKeys (and the loop in batchSyncUserConfigToKeys) to call this
helper to remove duplication and keep mapping logic in one place.
- Around line 284-303: The helper buildUserDbUpdates only handles a subset of
user fields synced to keys but UpdateUserSchema (used in syncUserConfigToKeys)
can accept extra fields like limit5hResetMode which buildUserDbUpdates currently
ignores and thus silently drops (and also misses the Redis 5h cache cleanup
logic in batchUpdateUsers). Either: add a clear comment at the top of
buildUserDbUpdates stating it only processes the subset of fields that are
synced to keys and that user-only fields (e.g., limit5hResetMode) are
intentionally not handled here; or better, derive a narrower Zod/TypeScript
schema via pick() for syncUserConfigToKeys that only permits the keys
buildUserDbUpdates expects (so unknown fields are rejected), and if you intend
to support limit5hResetMode here, add explicit handling in buildUserDbUpdates
plus the corresponding Redis cache cleanup logic used by batchUpdateUsers.
Ensure references: EditableUserData, buildUserDbUpdates, syncUserConfigToKeys,
UpdateUserSchema, batchUpdateUsers are updated accordingly.
In `@src/app/`[locale]/dashboard/_components/user/forms/key-edit-section.tsx:
- Around line 469-542: The JSX repeats "showProviderGroup &&" across several
ternary branches; refactor by extracting a single outer guard: wrap the entire
multi-branch expression with showProviderGroup && ( ... ) and inside keep the
existing nested ternaries/JSX for the isAdmin branch (ProviderGroupSelect), the
userGroups branch (read-only Badge list when keyData.id > 0 and TagInputField
when creating), the keyGroupOptions branch, and the fallback hint; ensure you
reference and preserve logic around isAdmin, ProviderGroupSelect, TagInputField,
keyGroupOptions, userGroups and keyData.id so behavior and props
(onChange/handleUserProviderGroupChange/translations) remain unchanged.
In `@src/lib/users/user-key-sync.ts`:
- Around line 183-185: buildFirstSyncedKeyConfig currently returns
buildSyncedKeyConfigs(source, 1).configs[0] which assumes configs[0] exists; to
make this safe change buildFirstSyncedKeyConfig to defensively handle missing
element by checking the result of buildSyncedKeyConfigs(source, 1).configs and
throwing a clear error or asserting when configs[0] is undefined, ensuring the
function always either returns a valid SyncedKeyConfig or fails fast (reference:
buildFirstSyncedKeyConfig, buildSyncedKeyConfigs, SyncedKeyConfig, configs).
- Around line 70-100: The file lacks a top-level JSDoc clarifying divergent
semantics between monetary and session distribution: update the header of
src/lib/users/user-key-sync.ts to document that distributeAmount (used for
limit*Usd fields) intentionally gives the first N keys 0.01 when totalCents <
keyCount and sets the remaining keys to null to mean "no limit", whereas
distributeConcurrentSessions (and limitConcurrentSessions) treats remaining keys
as 0 (explicit zero capacity); mention the exact fields (limit*Usd and
limitConcurrentSessions), the functions distributeAmount and
distributeConcurrentSessions, and that this is intentional to avoid future
confusion.
In `@tests/unit/user-dialogs.test.tsx`:
- Line 350: Replace the hardcoded button label check
expect(buttonTexts).toContain("Sync to Keys"); with the i18n messages constant
used elsewhere—use messages.dashboard.userManagement.editDialog.reset5h.button
(the same symbol used in other assertions) so the test references the localized
string instead of a literal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 769b0fc0-0a6d-46bf-b445-ee48210254b0
📒 Files selected for processing (23)
messages/en/dashboard.jsonmessages/en/settings/config.jsonmessages/ja/dashboard.jsonmessages/ja/settings/config.jsonmessages/ru/dashboard.jsonmessages/ru/settings/config.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/config.jsonsrc/actions/users.tssrc/app/[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsxsrc/app/[locale]/dashboard/_components/user/batch-edit/batch-user-section.tsxsrc/app/[locale]/dashboard/_components/user/create-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/edit-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/lib/users/user-key-sync.tstests/unit/actions/users-key-sync.test.tstests/unit/actions/users-reset-all-statistics.test.tstests/unit/lib/users/user-key-sync.test.tstests/unit/user-dialogs.test.tsx
| "syncKeys": "Sync to Keys", | ||
| "syncKeysDescription": "This will overwrite all undeleted keys for these {users} users from their current user settings.", |
There was a problem hiding this comment.
日文语言包中混入英文文案，建议统一为日文。
这些键会直接展示给 ja 用户，当前英文整句会导致界面语言不一致。
建议修复
- "syncKeys": "Sync to Keys",
- "syncKeysDescription": "This will overwrite all undeleted keys for these {users} users from their current user settings.",
+ "syncKeys": "キーに同期",
+ "syncKeysDescription": "選択した {users} ユーザーの未削除キーを、現在のユーザー設定で上書きします。",
@@
- "keysSynced": "Synced {keys} keys for {users} users",
+ "keysSynced": "{users} ユーザーの {keys} 個のキーを同期しました",
@@
- "syncFailed": "Sync to keys failed: {error}",
+ "syncFailed": "キーへの同期に失敗しました: {error}",
@@
- "syncKeys": "Sync to Keys"
+ "syncKeys": "キーに同期"
@@
- "syncKeysDescription": "When enabled, user fields from this batch edit are saved first, then all undeleted keys for each user are overwritten from that user's settings."
+ "syncKeysDescription": "有効にすると、この一括編集のユーザー項目を先に保存し、その後各ユーザーの未削除キーをユーザー設定で上書きします。"Also applies to: 1694-1697, 1720-1720, 1726-1726
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ja/dashboard.json` around lines 1684 - 1685, The Japanese locale
file contains English text for the keys "syncKeys" and "syncKeysDescription"
(and additional English strings around the same area at the other noted
entries), so replace those English values with proper Japanese translations to
keep UI language consistent; locate the keys "syncKeys", "syncKeysDescription"
and the other affected keys in messages/ja/dashboard.json (the entries reported
at the nearby blocks) and update their string values to Japanese equivalents
that convey the meaning ("同期するキー" etc.), ensuring placeholders like {users}
remain intact and punctuation/escaping matches the JSON style used elsewhere.
| const keyRows = await tx | ||
| .select({ | ||
| id: keysTable.id, | ||
| key: keysTable.key, | ||
| userId: keysTable.userId, | ||
| }) | ||
| .from(keysTable) | ||
| .where(and(inArray(keysTable.userId, requestedIds), isNull(keysTable.deletedAt))) | ||
| .orderBy(asc(keysTable.userId), asc(keysTable.createdAt), asc(keysTable.id)); | ||
|
|
||
| const keysByUserId = new Map<number, typeof keyRows>(); | ||
| for (const keyRow of keyRows) { | ||
| const rows = keysByUserId.get(keyRow.userId) ?? []; | ||
| rows.push(keyRow); | ||
| keysByUserId.set(keyRow.userId, rows); | ||
| } | ||
|
|
||
| const userResults: SyncUserConfigToKeysResult[] = []; | ||
|
|
||
| for (const userRow of userRows) { | ||
| const userKeys = keysByUserId.get(userRow.id) ?? []; | ||
| const { configs, summary } = buildSyncedKeyConfigs( | ||
| { | ||
| dailyQuota: userRow.dailyQuota, | ||
| limit5hUsd: userRow.limit5hUsd, | ||
| limitWeeklyUsd: userRow.limitWeeklyUsd, | ||
| limitMonthlyUsd: userRow.limitMonthlyUsd, | ||
| limitTotalUsd: userRow.limitTotalUsd, | ||
| limitConcurrentSessions: userRow.limitConcurrentSessions, | ||
| providerGroup: userRow.providerGroup, | ||
| dailyResetMode: userRow.dailyResetMode, | ||
| dailyResetTime: userRow.dailyResetTime, | ||
| }, | ||
| userKeys.length | ||
| ); | ||
|
|
||
| for (const [index, keyRow] of userKeys.entries()) { | ||
| const config = configs[index]; | ||
| await tx | ||
| .update(keysTable) | ||
| .set({ | ||
| updatedAt: new Date(), | ||
| limit5hUsd: config.limit5hUsd === null ? null : config.limit5hUsd.toString(), | ||
| limitDailyUsd: config.limitDailyUsd === null ? null : config.limitDailyUsd.toString(), | ||
| dailyResetMode: config.dailyResetMode, | ||
| dailyResetTime: config.dailyResetTime, | ||
| limitWeeklyUsd: | ||
| config.limitWeeklyUsd === null ? null : config.limitWeeklyUsd.toString(), | ||
| limitMonthlyUsd: | ||
| config.limitMonthlyUsd === null ? null : config.limitMonthlyUsd.toString(), | ||
| limitTotalUsd: config.limitTotalUsd === null ? null : config.limitTotalUsd.toString(), | ||
| limitConcurrentSessions: config.limitConcurrentSessions, | ||
| providerGroup: config.providerGroup, | ||
| }) | ||
| .where(and(eq(keysTable.id, keyRow.id), isNull(keysTable.deletedAt))); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查仓库是否已有 CASE WHEN 批量更新的工具/先例,以决定参考写法
rg -nP --type=ts -C2 "CASE\s+WHEN" -g 'src/**'
rg -nP --type=ts -C2 "sql\`" -g 'src/repository/**'Repository: ding113/claude-code-hub
Length of output: 50379
🏁 Script executed:
cd src/actions && wc -l users.ts && sed -n '1382,1437p' users.tsRepository: ding113/claude-code-hub
Length of output: 2513
🏁 Script executed:
# Check for MAX_BATCH_SIZE definition
rg -n "MAX_BATCH_SIZE" src/Repository: ding113/claude-code-hub
Length of output: 751
🏁 Script executed:
# Check the full function context to understand the batch processing
sed -n '1350,1450p' src/actions/users.tsRepository: ding113/claude-code-hub
Length of output: 4124
性能问题：批量同步在事务内对每把 Key 串行发 UPDATE，大批量场景下事务持有时间过长
batchSyncUserConfigToKeys 在事务中对每个用户的每把 Key 顺序发起单独 UPDATE（第 1418-1437 行）。MAX_BATCH_SIZE=500，若平均每用户 3-5 把 Key，合计 1500-2500 次串行 round-trip 在同一事务连接上执行，容易触发：
- 连接级别的 statement timeout / 事务空闲超时
- 长事务对
keys表上其它写操作的锁等待 - 进程被取消后已提交部分可能丢失
建议采用 Drizzle 的 sql 模板 + CASE WHEN id THEN ... END 的批量更新模式（参考已有的 src/repository/provider.ts:954-964 和 src/repository/message-write-buffer.ts:116-160），将事务内的 N 次 UPDATE 降低到 O(字段数) 的规模，显著降低事务持有时间和锁竞争风险。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/users.ts` around lines 1382 - 1437, The current
batchSyncUserConfigToKeys logic issues a separate tx.update for each key (loop
around keysTable updates), causing many serial round-trips and long
transactions; replace the per-key updates inside batchSyncUserConfigToKeys by
building a single batched UPDATE using Drizzle's sql template with CASE WHEN
keyed by keysTable.id (similar to the pattern in src/repository/provider.ts
lines ~954-964 and src/repository/message-write-buffer.ts lines ~116-160):
collect all key ids and for each updatable column (limit5hUsd, limitDailyUsd,
limitWeeklyUsd, limitMonthlyUsd, limitTotalUsd, limitConcurrentSessions,
providerGroup, dailyResetMode, dailyResetTime, updatedAt) build CASE expressions
mapping id -> value (use NULL where appropriate), then execute one tx.sql`UPDATE
${keysTable} SET ... WHERE ${inArray(keysTable.id, ids)} AND
${isNull(keysTable.deletedAt)}` inside the transaction to replace the looped
tx.update calls.
dofastted
force-pushed
the
pr/dev-user-key-sync-20260426
branch
from
588a923 to
c7b774f
Compare
|
Updated after review:\n\n- user-to-key sync now includes for single-user sync, batch sync, and default key creation.\n- validation helper import moved to the module top level.\n- cache invalidation now runs as a background task with warning logs on failure.\n- batch sync error logging and fallback user-facing error now follow the existing English log / translated error pattern.\n\nVerification after update:\n- stdout | tests/unit/lib/users/user-key-sync.test.ts Vitest 测试环境初始化... 测试配置:
stdout | tests/unit/lib/users/user-key-sync.test.ts Vitest 测试环境清理... Vitest 测试环境清理完成 ✓ |0| tests/unit/lib/users/user-key-sync.test.ts > user key sync allocation > single key gets the same user limits 1ms Vitest 测试环境初始化... 测试配置:
stdout | tests/unit/user-dialogs.test.tsx Vitest 测试环境初始化... 测试配置:
✓ |0| tests/unit/user-dialogs.test.tsx > EditUserDialog > renders dialog with user data when open 43ms Vitest 测试环境清理... Vitest 测试环境清理完成 ✓ |0| tests/unit/user-dialogs.test.tsx > EditUserDialog > EditUserDialog disables reset 5h button when no 5h limit is configured 14ms Vitest 测试环境清理... Vitest 测试环境清理完成 ✓ |0| tests/unit/actions/users-key-sync.test.ts > users key sync actions > syncUserConfigToKeys saves user and updates all undeleted keys by created order 4ms Test Files 3 passed (3)
|
|
Correction for the previous comment formatting. Updated after review:\n\n- user-to-key sync now includes limit5hResetMode for single-user sync, batch sync, and default key creation.\n- validation helper import moved to the module top level.\n- cache invalidation now runs as a background task with warning logs on failure.\n- batch sync error logging and fallback user-facing error now follow the existing English log / translated error pattern.\n\nVerification after update:\n- Vitest targeted user-key sync tests passed with testTimeout=30000.\n- npm run typecheck passed. |
| if (normalizedTotal >= keyCount) { | ||
| const perKey = Math.floor(normalizedTotal / keyCount); | ||
| return { | ||
| values: Array.from({ length: keyCount }, () => perKey), | ||
| discarded: normalizedTotal - perKey * keyCount, | ||
| }; | ||
| } |
There was a problem hiding this comment.
When normalizedTotal >= keyCount, the floor-division remainder is computed but never distributed — all keys get perKey sessions and the leftover is discarded. For example, 7 sessions across 3 keys produces [2, 2, 2] (total: 6) instead of [3, 2, 2] (total: 7). The PR description explicitly states "remainder distributed one-per-key to earliest keys", but this branch doesn't implement that. The test suite only exercises normalizedTotal < keyCount, so this path is untested.
| if (normalizedTotal >= keyCount) { | |
| const perKey = Math.floor(normalizedTotal / keyCount); | |
| return { | |
| values: Array.from({ length: keyCount }, () => perKey), | |
| discarded: normalizedTotal - perKey * keyCount, | |
| }; | |
| } | |
| if (normalizedTotal >= keyCount) { | |
| const perKey = Math.floor(normalizedTotal / keyCount); | |
| const remainder = normalizedTotal - perKey * keyCount; | |
| return { | |
| values: Array.from({ length: keyCount }, (_, index) => | |
| index < remainder ? perKey + 1 : perKey | |
| ), | |
| discarded: 0, | |
| }; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/users/user-key-sync.ts
Line: 117-123
Comment:
**Concurrent session remainder silently dropped**
When `normalizedTotal >= keyCount`, the floor-division remainder is computed but never distributed — all keys get `perKey` sessions and the leftover is discarded. For example, 7 sessions across 3 keys produces `[2, 2, 2]` (total: 6) instead of `[3, 2, 2]` (total: 7). The PR description explicitly states *"remainder distributed one-per-key to earliest keys"*, but this branch doesn't implement that. The test suite only exercises `normalizedTotal < keyCount`, so this path is untested.
```suggestion
if (normalizedTotal >= keyCount) {
const perKey = Math.floor(normalizedTotal / keyCount);
const remainder = normalizedTotal - perKey * keyCount;
return {
values: Array.from({ length: keyCount }, (_, index) =>
index < remainder ? perKey + 1 : perKey
),
discarded: 0,
};
}
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/app/[locale]/dashboard/_components/user/create-user-dialog.tsx (1)
175-185:⚠️ Potential issue | 🟠 Major首个 Key 同步仍使用表单草稿，可能与已落库用户配置不一致。
Line 175 至 Line 185 仍从
data.user构建firstKeyConfig，Line 197 也未使用firstKeyConfig.limit5hResetMode。当服务端在createUserOnly后补齐/规范化默认值时，首个 Key 会和真实用户配置偏离。建议修复
- const firstKeyConfig = buildFirstSyncedKeyConfig({ - dailyQuota: data.user.dailyQuota ?? null, - limit5hUsd: data.user.limit5hUsd ?? null, - limitWeeklyUsd: data.user.limitWeeklyUsd ?? null, - limitMonthlyUsd: data.user.limitMonthlyUsd ?? null, - limitTotalUsd: data.user.limitTotalUsd ?? null, - limitConcurrentSessions: data.user.limitConcurrentSessions ?? null, - providerGroup: data.user.providerGroup ?? PROVIDER_GROUP.DEFAULT, - dailyResetMode: data.user.dailyResetMode, - dailyResetTime: data.user.dailyResetTime, - }); + const persistedUser = userRes.data.user; + const firstKeyConfig = buildFirstSyncedKeyConfig({ + dailyQuota: persistedUser.dailyQuota ?? null, + limit5hUsd: persistedUser.limit5hUsd ?? null, + limitWeeklyUsd: persistedUser.limitWeeklyUsd ?? null, + limitMonthlyUsd: persistedUser.limitMonthlyUsd ?? null, + limitTotalUsd: persistedUser.limitTotalUsd ?? null, + limitConcurrentSessions: persistedUser.limitConcurrentSessions ?? null, + providerGroup: persistedUser.providerGroup ?? PROVIDER_GROUP.DEFAULT, + limit5hResetMode: persistedUser.limit5hResetMode ?? null, + dailyResetMode: persistedUser.dailyResetMode ?? null, + dailyResetTime: persistedUser.dailyResetTime ?? null, + }); @@ - limit5hResetMode: data.user.limit5hResetMode ?? data.key.limit5hResetMode, + limit5hResetMode: firstKeyConfig.limit5hResetMode,Also applies to: 197-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/create-user-dialog.tsx around lines 175 - 185, The code builds firstKeyConfig from the form draft (data.user) which can differ from the server-normalized user after createUserOnly; change the logic to construct firstKeyConfig from the server-returned/created user object (the response from createUserOnly) rather than data.user, and ensure you actually use firstKeyConfig.limit5hResetMode where intended (replace any direct references to data.user.limit5hResetMode or hardcoded draft values with firstKeyConfig.limit5hResetMode). Locate usages around buildFirstSyncedKeyConfig, firstKeyConfig, and createUserOnly and update them so the first key uses the persisted/normalized user fields.messages/ja/dashboard.json (1)
1620-1625:⚠️ Potential issue | 🟡 Minor日文文案里仍混入了英文
Key。这组提示会直接展示给 ja 用户，当前的
Key会让这块 UI 语言不一致，建议统一改成日文表述（例如キー）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/ja/dashboard.json` around lines 1620 - 1625, The Japanese copy in the syncKeys object still contains the English word "Key"; update all occurrences in the syncKeys fields ("button", "loading", "success", "error") to use the Japanese term "キー" (e.g., change "Key に同期" → "キーに同期", "{count} 個の Key に同期しました" → "{count} 個の キーに同期しました" or adjust spacing to match project style) so the UI text is fully Japanese and consistent.src/actions/users.ts (1)
1439-1458:⚠️ Potential issue | 🟠 Major批量同步仍在事务里按 Key 串行执行单行
UPDATE。这里在 500 用户上限下会把事务拉长成大量逐 key 的往返更新，容易放大锁等待和 statement timeout 风险。更稳妥的做法是把同一批 key 合并成批量更新（例如按列构造
CASE WHEN id THEN ... END），至少不要在事务里逐条写。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 1439 - 1458, The loop over userKeys (and configs) issues one awaited tx.update per key which prolongs the transaction and causes many round-trips; refactor the update logic in the function that uses userKeys/configs to perform a single batched update instead of per-key updates: collect ids and for each updatable column build a CASE WHEN ... THEN ... END expression (or use your DB client's bulk upsert/batch update API) and call tx.update(keysTable).set({...CASE expressions...}).where(in(keysTable.id, ids)) so you remove the per-iteration await and collapse all key updates into one statement within the transaction (or move the batch update outside the transaction if appropriate). Ensure you reference userKeys, configs, tx.update(keysTable), and keysTable.id when implementing the change.src/app/[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsx (1)
441-448:⚠️ Potential issue | 🟠 Major确认框仍会低估这次操作会改动的 Key 数量。
当只开启“同步到 Keys”而没有单独选中 key 字段时，这里仍把
keysCount算成0，但下面的说明又写着会覆盖这些用户的全部未删除 key。确认信息不能比真实影响范围更小；这里至少应改成不显示具体 key 数，或单独走“同步这些用户下全部未删除 key”的文案分支。Also applies to: 458-463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsx around lines 441 - 448, The confirmation text underestimates affected keys because keysCount is set to 0 when willUpdateKeys is false even if willSyncUsersToKeys is true; update the logic around keysCount and the rendering branch: compute keysCount only when willUpdateKeys is true (keep keysCount = pendingUpdate.keyIds.length) but add a separate branch/flag (e.g., isSyncAllKeys = willSyncUsersToKeys && !willUpdateKeys) and when isSyncAllKeys is true render a different translation/message (or omit numeric keys parameter) that says "sync all undeleted keys for these users" instead of showing 0, using the existing symbols willSyncUsersToKeys, willUpdateKeys, keysCount, usersCount and pendingUpdate.keyIds in the component that renders t("confirm.description") (and the other occurrence around lines 458–463).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/ru/dashboard.json`:
- Around line 1689-1690: The Russian messages file contains English text for
several i18n keys (e.g., "syncKeys", "syncKeysDescription" and other keys added
between lines ~1689–1731); replace those English strings with their Russian
translations in messages/ru/dashboard.json so the ru locale is fully localized
(do not hardcode display text elsewhere), ensuring keys like syncKeys,
syncKeysDescription and the other mentioned keys (around 1699–1702, 1725–1726,
1731) contain proper Russian text consistent with the other locales.
In `@src/actions/users.ts`:
- Around line 96-108: The helper invalidateUserKeySyncCaches currently
fire-and-forgets invalidation via void Promise.all(...), causing callers like
syncUserConfigToKeys and batchSyncUserConfigToKeys to return before Redis
invalidation completes; change invalidateUserKeySyncCaches to return
Promise<void> (remove void and return the Promise) and preserve the existing
per-item .catch logging (warnUserCacheInvalidationFailure /
warnKeyCacheInvalidationFailure), then update the two callers to await
invalidateUserKeySyncCaches("user-key sync", [userId], keyStringsForCache) and
await invalidateUserKeySyncCaches("batch user-key sync", requestedIds,
keyStringsForCache) so that cache invalidation (invalidateCachedUser /
invalidateCachedKey) completes before success is returned.
---
Duplicate comments:
In `@messages/ja/dashboard.json`:
- Around line 1620-1625: The Japanese copy in the syncKeys object still contains
the English word "Key"; update all occurrences in the syncKeys fields ("button",
"loading", "success", "error") to use the Japanese term "キー" (e.g., change "Key
に同期" → "キーに同期", "{count} 個の Key に同期しました" → "{count} 個の キーに同期しました" or adjust
spacing to match project style) so the UI text is fully Japanese and consistent.
In `@src/actions/users.ts`:
- Around line 1439-1458: The loop over userKeys (and configs) issues one awaited
tx.update per key which prolongs the transaction and causes many round-trips;
refactor the update logic in the function that uses userKeys/configs to perform
a single batched update instead of per-key updates: collect ids and for each
updatable column build a CASE WHEN ... THEN ... END expression (or use your DB
client's bulk upsert/batch update API) and call
tx.update(keysTable).set({...CASE expressions...}).where(in(keysTable.id, ids))
so you remove the per-iteration await and collapse all key updates into one
statement within the transaction (or move the batch update outside the
transaction if appropriate). Ensure you reference userKeys, configs,
tx.update(keysTable), and keysTable.id when implementing the change.
In
`@src/app/`[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsx:
- Around line 441-448: The confirmation text underestimates affected keys
because keysCount is set to 0 when willUpdateKeys is false even if
willSyncUsersToKeys is true; update the logic around keysCount and the rendering
branch: compute keysCount only when willUpdateKeys is true (keep keysCount =
pendingUpdate.keyIds.length) but add a separate branch/flag (e.g., isSyncAllKeys
= willSyncUsersToKeys && !willUpdateKeys) and when isSyncAllKeys is true render
a different translation/message (or omit numeric keys parameter) that says "sync
all undeleted keys for these users" instead of showing 0, using the existing
symbols willSyncUsersToKeys, willUpdateKeys, keysCount, usersCount and
pendingUpdate.keyIds in the component that renders t("confirm.description") (and
the other occurrence around lines 458–463).
In `@src/app/`[locale]/dashboard/_components/user/create-user-dialog.tsx:
- Around line 175-185: The code builds firstKeyConfig from the form draft
(data.user) which can differ from the server-normalized user after
createUserOnly; change the logic to construct firstKeyConfig from the
server-returned/created user object (the response from createUserOnly) rather
than data.user, and ensure you actually use firstKeyConfig.limit5hResetMode
where intended (replace any direct references to data.user.limit5hResetMode or
hardcoded draft values with firstKeyConfig.limit5hResetMode). Locate usages
around buildFirstSyncedKeyConfig, firstKeyConfig, and createUserOnly and update
them so the first key uses the persisted/normalized user fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a5e4ab3-2afc-41a6-99fd-8b9f22e3bdd3
📒 Files selected for processing (23)
messages/en/dashboard.jsonmessages/en/settings/config.jsonmessages/ja/dashboard.jsonmessages/ja/settings/config.jsonmessages/ru/dashboard.jsonmessages/ru/settings/config.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/config.jsonsrc/actions/users.tssrc/app/[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsxsrc/app/[locale]/dashboard/_components/user/batch-edit/batch-user-section.tsxsrc/app/[locale]/dashboard/_components/user/create-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/edit-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/lib/users/user-key-sync.tstests/unit/actions/users-key-sync.test.tstests/unit/actions/users-reset-all-statistics.test.tstests/unit/lib/users/user-key-sync.test.tstests/unit/user-dialogs.test.tsx
✅ Files skipped from review due to trivial changes (11)
- messages/en/settings/config.json
- messages/ja/settings/config.json
- tests/unit/actions/users-reset-all-statistics.test.ts
- src/app/[locale]/settings/config/page.tsx
- tests/unit/lib/users/user-key-sync.test.ts
- src/app/[locale]/dashboard/_components/user/batch-edit/batch-user-section.tsx
- messages/zh-CN/settings/config.json
- messages/ru/settings/config.json
- messages/en/dashboard.json
- src/app/[locale]/dashboard/_components/user/edit-user-dialog.tsx
- src/lib/users/user-key-sync.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- messages/zh-TW/settings/config.json
| "syncKeys": "Sync to Keys", | ||
| "syncKeysDescription": "This will overwrite all undeleted keys for these {users} users from their current user settings.", |
There was a problem hiding this comment.
俄语语言包中出现英文文案，批量同步流程会出现混合语言。
Line 1689 至 Line 1731 的新增键值仍是英文（如 Sync to Keys、This will overwrite...），这会直接影响俄语用户的关键操作确认与错误提示体验。
建议修复（俄语示例）
- "syncKeys": "Sync to Keys",
- "syncKeysDescription": "This will overwrite all undeleted keys for these {users} users from their current user settings.",
+ "syncKeys": "Синхронизировать в ключи",
+ "syncKeysDescription": "Это перезапишет все неудалённые ключи для {users} пользователей на основе их текущих настроек пользователя.",
@@
- "keysSynced": "Synced {keys} keys for {users} users",
+ "keysSynced": "Синхронизировано {keys} ключей для {users} пользователей",
@@
- "syncFailed": "Sync to keys failed: {error}",
+ "syncFailed": "Синхронизация в ключи не удалась: {error}",
@@
- "syncKeys": "Sync to Keys"
+ "syncKeys": "Синхронизировать в ключи"
@@
- "syncKeysDescription": "When enabled, user fields from this batch edit are saved first, then all undeleted keys for each user are overwritten from that user's settings."
+ "syncKeysDescription": "При включении сначала сохраняются поля пользователя из этого массового редактирования, затем для каждого пользователя перезаписываются все неудалённые ключи по его настройкам."As per coding guidelines "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text".
Also applies to: 1699-1702, 1725-1726, 1731-1731
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ru/dashboard.json` around lines 1689 - 1690, The Russian messages
file contains English text for several i18n keys (e.g., "syncKeys",
"syncKeysDescription" and other keys added between lines ~1689–1731); replace
those English strings with their Russian translations in
messages/ru/dashboard.json so the ru locale is fully localized (do not hardcode
display text elsewhere), ensuring keys like syncKeys, syncKeysDescription and
the other mentioned keys (around 1699–1702, 1725–1726, 1731) contain proper
Russian text consistent with the other locales.
| function invalidateUserKeySyncCaches(context: string, userIds: number[], keyStrings: string[]) { | ||
| void Promise.all([ | ||
| ...userIds.map((userId) => | ||
| invalidateCachedUser(userId).catch((error) => | ||
| warnUserCacheInvalidationFailure(context, userId, error) | ||
| ) | ||
| ), | ||
| ...keyStrings.map((key) => | ||
| invalidateCachedKey(key).catch((error) => | ||
| warnKeyCacheInvalidationFailure(context, key, error) | ||
| ) | ||
| ), | ||
| ]); |
There was a problem hiding this comment.
不要把鉴权缓存失效做成 fire-and-forget。
这里直接 void Promise.all(...)，会让 syncUserConfigToKeys / batchSyncUserConfigToKeys 在 Redis 失效真正完成前就返回成功。下一次请求如果刚好命中旧缓存，用户或 Key 仍可能短时间继续使用旧额度/旧 providerGroup，和“同步成功”的语义不一致。建议让这个 helper 返回 Promise<void>，并在两个 action 的成功路径里显式 await。
建议修改
-function invalidateUserKeySyncCaches(context: string, userIds: number[], keyStrings: string[]) {
- void Promise.all([
+async function invalidateUserKeySyncCaches(
+ context: string,
+ userIds: number[],
+ keyStrings: string[]
+) {
+ await Promise.all([
...userIds.map((userId) =>
invalidateCachedUser(userId).catch((error) =>
warnUserCacheInvalidationFailure(context, userId, error)
@@
...keyStrings.map((key) =>
invalidateCachedKey(key).catch((error) =>
warnKeyCacheInvalidationFailure(context, key, error)
)
),
]);
}并把两个调用点改成：
await invalidateUserKeySyncCaches("batch user-key sync", requestedIds, keyStringsForCache);
await invalidateUserKeySyncCaches("user-key sync", [userId], keyStringsForCache);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/users.ts` around lines 96 - 108, The helper
invalidateUserKeySyncCaches currently fire-and-forgets invalidation via void
Promise.all(...), causing callers like syncUserConfigToKeys and
batchSyncUserConfigToKeys to return before Redis invalidation completes; change
invalidateUserKeySyncCaches to return Promise<void> (remove void and return the
Promise) and preserve the existing per-item .catch logging
(warnUserCacheInvalidationFailure / warnKeyCacheInvalidationFailure), then
update the two callers to await invalidateUserKeySyncCaches("user-key sync",
[userId], keyStringsForCache) and await invalidateUserKeySyncCaches("batch
user-key sync", requestedIds, keyStringsForCache) so that cache invalidation
(invalidateCachedUser / invalidateCachedKey) completes before success is
returned.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7b774f355
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| limit5hUsd: data.limit5hUsd, | ||
| dailyQuota: data.dailyQuota, | ||
| limitWeeklyUsd: data.limitWeeklyUsd, |
There was a problem hiding this comment.
The new handleSyncKeys request omits limit5hResetMode, so clicking Sync to Keys after changing the 5h window mode in the edit form will keep using the old mode from the database and propagate that stale mode to keys. This creates a silent mismatch where the UI shows a new mode but the sync operation does not apply it; include limit5hResetMode in this payload (as the normal save path already does).
Useful? React with 👍 / 👎.
| if (normalizedTotal >= keyCount) { | ||
| const perKey = Math.floor(normalizedTotal / keyCount); | ||
| return { | ||
| values: Array.from({ length: keyCount }, () => perKey), | ||
| discarded: normalizedTotal - perKey * keyCount, |
There was a problem hiding this comment.
distributeConcurrentSessions drops the remainder whenever normalizedTotal >= keyCount (discarded is returned but never reassigned), so a user limit like 5 sessions across 3 keys becomes [1,1,1] instead of preserving all 5 sessions. This systematically under-allocates concurrency after sync for many non-divisible totals, causing avoidable throttling versus the configured user limit.
Useful? React with 👍 / 👎.
dofastted
force-pushed
the
pr/dev-user-key-sync-20260426
branch
from
c7b774f to
a66fbe9
Compare
|
Second review update pushed. Additional changes:\n\n- Create-user flow now builds the first key config from the saved user returned by the server, not only from the local form draft.\n- Edit-user sync path now passes limit5hResetMode.\n- createUserOnly response now returns daily reset fields needed by the first-key sync path.\n- Japanese dashboard sync strings and Russian billing correction strings are localized.\n\nVerification:\n- Targeted user-key sync Vitest passed: 27 passed.\n- npm run typecheck passed. |
| const limit5hResetMode: "fixed" | "rolling" = | ||
| source.limit5hResetMode === "fixed" ? "fixed" : "rolling"; | ||
| const dailyResetMode: "fixed" | "rolling" = | ||
| source.dailyResetMode === "rolling" ? "rolling" : "fixed"; | ||
| const dailyResetTime = source.dailyResetTime || "00:00"; |
There was a problem hiding this comment.
limit5hResetMode defaults to "rolling" when the source value is absent/null (=== "fixed" ? "fixed" : "rolling"), while dailyResetMode defaults to "fixed" (=== "rolling" ? "rolling" : "fixed"). These two guards use opposite logic, so a user row where neither reset mode has been explicitly set will have its keys synced with limit5hResetMode: "rolling" and dailyResetMode: "fixed". If the intended fallback for both fields is the same value (e.g., both "rolling" or both "fixed"), one of these expressions is inverted.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/users/user-key-sync.ts
Line: 137-141
Comment:
**Asymmetric defaults for reset modes**
`limit5hResetMode` defaults to `"rolling"` when the source value is absent/null (`=== "fixed" ? "fixed" : "rolling"`), while `dailyResetMode` defaults to `"fixed"` (`=== "rolling" ? "rolling" : "fixed"`). These two guards use opposite logic, so a user row where neither reset mode has been explicitly set will have its keys synced with `limit5hResetMode: "rolling"` and `dailyResetMode: "fixed"`. If the intended fallback for both fields is the same value (e.g., both "rolling" or both "fixed"), one of these expressions is inverted.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
messages/ja/dashboard.json (1)
1720-1726:⚠️ Potential issue | 🟠 Major日语语言包这里仍有英文文案，批量同步流程会出现混合语言。
Line 1720 和 Line 1726 仍是英文，建议改成日文以保持
ja体验一致。As per coding guidelines "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text".建议修复
- "syncKeys": "Sync to Keys" + "syncKeys": "キーに同期" @@ - "syncKeysDescription": "When enabled, user fields from this batch edit are saved first, then all undeleted keys for each user are overwritten from that user's settings." + "syncKeysDescription": "有効にすると、この一括編集のユーザー項目を先に保存し、その後各ユーザーの未削除キーをユーザー設定で上書きします。"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/ja/dashboard.json` around lines 1720 - 1726, The ja locale contains English strings for the batch-sync UI keys "syncKeys" and "syncKeysDescription"; update messages/ja/dashboard.json by replacing the values for "syncKeys" and "syncKeysDescription" with appropriate Japanese translations (keeping the key names unchanged) so the batch edit UI is fully localized in Japanese and follows i18n guidelines.messages/ru/dashboard.json (1)
1689-1731:⚠️ Potential issue | 🟠 Major俄语语言包中这组新增文案仍是英文，会导致关键流程混合语言。
syncKeys相关确认提示、toast、字段描述在 ru locale 下仍未本地化，建议统一为俄语。As per coding guidelines "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text".建议修复
- "syncKeys": "Sync to Keys", - "syncKeysDescription": "This will overwrite all undeleted keys for these {users} users from their current user settings.", + "syncKeys": "Синхронизировать в ключи", + "syncKeysDescription": "Это перезапишет все неудалённые ключи для {users} пользователей на основе их текущих настроек пользователя.", @@ - "keysSynced": "Synced {keys} keys for {users} users", + "keysSynced": "Синхронизировано {keys} ключей для {users} пользователей", @@ - "syncFailed": "Sync to keys failed: {error}", + "syncFailed": "Не удалось синхронизировать в ключи: {error}", @@ - "syncKeys": "Sync to Keys" + "syncKeys": "Синхронизировать в ключи" @@ - "syncKeysDescription": "When enabled, user fields from this batch edit are saved first, then all undeleted keys for each user are overwritten from that user's settings." + "syncKeysDescription": "При включении сначала сохраняются поля пользователя из этого массового редактирования, затем для каждого пользователя все неудалённые ключи перезаписываются его пользовательскими настройками."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/ru/dashboard.json` around lines 1689 - 1731, The ru locale contains several untranslated keys—update the string values for "syncKeys", "syncKeysDescription", "fields.syncKeys", "placeholders.syncKeysDescription", and the toast messages "toast.keysSynced" and "toast.syncFailed" to Russian; ensure you preserve interpolation tokens like {users}, {keys}, {error}, and {count} exactly as in the originals and keep JSON formatting/escaping correct so that syncKeys, syncKeysDescription, fields.syncKeys, placeholders.syncKeysDescription, toast.keysSynced, and toast.syncFailed are fully localized.
🧹 Nitpick comments (3)
src/app/[locale]/settings/config/_components/system-settings-form.tsx (1)
447-462: Optional：与卡片外开关风格对齐时可补一个图标容器。该开关行使用了
rounded-lg border border-white/5 bg-background/30 p-3的紧凑样式且未带图标容器，而Toggle Settings区块内的其他开关（如enable-http2、enable-thinking-signature-rectifier）统一为p-4 rounded-xl+ 彩色图标容器。当前布局是嵌套在已有顶部图标的billing-correction卡片内，差异是合理的；如果希望视觉上完全统一，可以补一个小尺寸图标容器（例如FileCode/Pencil），否则保持现状即可。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/config/_components/system-settings-form.tsx around lines 447 - 462, 当前在 system-settings-form.tsx 中与开关相关的卡片（enableBillingHeaderRectifier / Switch id="enable-billing-header-rectifier"）使用了紧凑的 p-3 rounded-lg 样式，与其它 Toggle Settings 中使用的 p-4 rounded-xl + 彩色图标容器不一致；若要视觉统一，请在该开关行左侧加入一个小尺寸图标容器（与其它开关相同的结构：占位圆角背景 + 图标组件，例如 FileCode 或 Pencil），并把外层容器样式调整为 p-4 rounded-xl（或在保持 border/bg 的同时增加左侧空间），确保 Switch 的 checked/onCheckedChange 逻辑（enableBillingHeaderRectifier, setEnableBillingHeaderRectifier）不变且 disabled={isPending} 保持；如果不需要统一外观，则无需改动。tests/unit/user-dialogs.test.tsx (1)
348-351: 避免在断言里硬编码同步按钮文案。Line 350 直接写死
"Sync to Keys"，后续文案或默认 locale 调整会让测试变脆。建议复用消息对象中的翻译键。建议修复
- expect(buttonTexts).toContain("Sync to Keys"); + expect(buttonTexts).toContain(messages.dashboard.userManagement.editDialog.syncKeys.button);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/user-dialogs.test.tsx` around lines 348 - 351, 测试中不应硬编码同步按钮文案：把 expect(buttonTexts).toContain("Sync to Keys") 替换为使用现有国际化消息/键（例如 messages.syncToKeys 或 调用 intl.formatMessage(messages.syncToKeys) / t('syncToKeys')），确保断言使用同一翻译源；在 user-dialogs.test.tsx 中定位 buttonTexts 断言并引入或复用测试上下文中的 messages/intl 实例来获取同步按钮的文案进行断言。src/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsx (1)
469-542: 可选重构：将showProviderGroup提到单一外层判断以减少重复。当前四个分支每个都重复
showProviderGroup &&，逻辑等价但稍显冗余。可以将其提到外层一次性判断，提升可读性，行为不变。♻️ 建议改写
- {showProviderGroup && isAdmin ? ( - <ProviderGroupSelect ... /> - ) : showProviderGroup && userGroups.length > 0 ? ( - <div className="space-y-2"> ... </div> - ) : showProviderGroup && keyGroupOptions.length > 0 ? ( - <div className="space-y-2"> ... </div> - ) : showProviderGroup ? ( - <div className="text-sm text-muted-foreground"> - {translations.fields.providerGroup.noGroupHint || "..."} - </div> - ) : null} + {showProviderGroup && + (isAdmin ? ( + <ProviderGroupSelect ... /> + ) : userGroups.length > 0 ? ( + <div className="space-y-2"> ... </div> + ) : keyGroupOptions.length > 0 ? ( + <div className="space-y-2"> ... </div> + ) : ( + <div className="text-sm text-muted-foreground"> + {translations.fields.providerGroup.noGroupHint || "..."} + </div> + ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/key-edit-section.tsx around lines 469 - 542, Refactor the JSX so the repeated "showProviderGroup &&" is checked once: wrap the entire current multi-branch expression with a single conditional on showProviderGroup and then keep the existing nested logic (isAdmin ? render ProviderGroupSelect : userGroups.length > 0 ? render read-only or TagInputField based on keyData.id : keyGroupOptions.length > 0 ? render badges : render noGroupHint). Ensure you preserve the exact components/props and text uses (ProviderGroupSelect, TagInputField, keyGroupOptions, userGroups, keyData, translations, PROVIDER_GROUP) and the read-only vs create-mode branches so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@messages/ja/dashboard.json`:
- Around line 1720-1726: The ja locale contains English strings for the
batch-sync UI keys "syncKeys" and "syncKeysDescription"; update
messages/ja/dashboard.json by replacing the values for "syncKeys" and
"syncKeysDescription" with appropriate Japanese translations (keeping the key
names unchanged) so the batch edit UI is fully localized in Japanese and follows
i18n guidelines.
In `@messages/ru/dashboard.json`:
- Around line 1689-1731: The ru locale contains several untranslated keys—update
the string values for "syncKeys", "syncKeysDescription", "fields.syncKeys",
"placeholders.syncKeysDescription", and the toast messages "toast.keysSynced"
and "toast.syncFailed" to Russian; ensure you preserve interpolation tokens like
{users}, {keys}, {error}, and {count} exactly as in the originals and keep JSON
formatting/escaping correct so that syncKeys, syncKeysDescription,
fields.syncKeys, placeholders.syncKeysDescription, toast.keysSynced, and
toast.syncFailed are fully localized.
---
Nitpick comments:
In `@src/app/`[locale]/dashboard/_components/user/forms/key-edit-section.tsx:
- Around line 469-542: Refactor the JSX so the repeated "showProviderGroup &&"
is checked once: wrap the entire current multi-branch expression with a single
conditional on showProviderGroup and then keep the existing nested logic
(isAdmin ? render ProviderGroupSelect : userGroups.length > 0 ? render read-only
or TagInputField based on keyData.id : keyGroupOptions.length > 0 ? render
badges : render noGroupHint). Ensure you preserve the exact components/props and
text uses (ProviderGroupSelect, TagInputField, keyGroupOptions, userGroups,
keyData, translations, PROVIDER_GROUP) and the read-only vs create-mode branches
so behavior remains identical.
In `@src/app/`[locale]/settings/config/_components/system-settings-form.tsx:
- Around line 447-462: 当前在 system-settings-form.tsx
中与开关相关的卡片（enableBillingHeaderRectifier / Switch
id="enable-billing-header-rectifier"）使用了紧凑的 p-3 rounded-lg 样式，与其它 Toggle
Settings 中使用的 p-4 rounded-xl +
彩色图标容器不一致；若要视觉统一，请在该开关行左侧加入一个小尺寸图标容器（与其它开关相同的结构：占位圆角背景 + 图标组件，例如 FileCode 或
Pencil），并把外层容器样式调整为 p-4 rounded-xl（或在保持 border/bg 的同时增加左侧空间），确保 Switch 的
checked/onCheckedChange 逻辑（enableBillingHeaderRectifier,
setEnableBillingHeaderRectifier）不变且 disabled={isPending} 保持；如果不需要统一外观，则无需改动。
In `@tests/unit/user-dialogs.test.tsx`:
- Around line 348-351: 测试中不应硬编码同步按钮文案：把 expect(buttonTexts).toContain("Sync to
Keys") 替换为使用现有国际化消息/键（例如 messages.syncToKeys 或 调用
intl.formatMessage(messages.syncToKeys) / t('syncToKeys')），确保断言使用同一翻译源；在
user-dialogs.test.tsx 中定位 buttonTexts 断言并引入或复用测试上下文中的 messages/intl
实例来获取同步按钮的文案进行断言。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a9779e2-27af-4c5f-8adb-331d00226340
📒 Files selected for processing (23)
messages/en/dashboard.jsonmessages/en/settings/config.jsonmessages/ja/dashboard.jsonmessages/ja/settings/config.jsonmessages/ru/dashboard.jsonmessages/ru/settings/config.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/config.jsonsrc/actions/users.tssrc/app/[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsxsrc/app/[locale]/dashboard/_components/user/batch-edit/batch-user-section.tsxsrc/app/[locale]/dashboard/_components/user/create-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/edit-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/lib/users/user-key-sync.tstests/unit/actions/users-key-sync.test.tstests/unit/actions/users-reset-all-statistics.test.tstests/unit/lib/users/user-key-sync.test.tstests/unit/user-dialogs.test.tsx
✅ Files skipped from review due to trivial changes (7)
- messages/ja/settings/config.json
- messages/en/dashboard.json
- src/app/[locale]/settings/config/page.tsx
- messages/zh-CN/settings/config.json
- messages/ru/settings/config.json
- src/app/[locale]/dashboard/_components/user/batch-edit/batch-user-section.tsx
- src/lib/users/user-key-sync.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- messages/zh-TW/settings/config.json
- tests/unit/actions/users-reset-all-statistics.test.ts
- tests/unit/lib/users/user-key-sync.test.ts
- messages/zh-TW/dashboard.json
- tests/unit/actions/users-key-sync.test.ts
- messages/en/settings/config.json
- src/actions/users.ts
- src/app/[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a66fbe93b6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| invalidateUserKeySyncCaches("user-key sync", [userId], keyStringsForCache); | ||
|
|
||
| revalidatePath("/dashboard"); |
There was a problem hiding this comment.
syncUserConfigToKeys writes limit5hResetMode updates and then only invalidates auth caches; unlike editUser/batchUpdateUsers, it never calls clearUserCostCache when the 5h mode flips. If an admin switches rolling↔fixed through the new Sync-to-Keys flow, Redis can retain mode-specific 5h counters from the previous mode, so subsequent limit checks may use stale/partial usage and mis-enforce quotas until TTL expiry. Please run the same reset-mode cache cleanup here (and in the batch sync path) whenever the mode changes.
Useful? React with 👍 / 👎.
|
您好，用户和 key 限额分离的设计是预期。用户、key、供应商三者限额最终的效果是取其小。 |
2 participants
Summary
Split from #1106 for focused review. Implements user-to-key configuration sync: propagating a user's quota, reset, provider-group, and concurrency settings to their existing keys, plus a settings page reorganization for billing-related controls.
Related to #1111 (sibling split for self-service quota windows).
Problem
When an admin changes a user's rate limits (daily quota, 5h/weekly/monthly/total spend caps, concurrent sessions, provider group, daily reset mode/time), those changes only apply to the user row. The user's existing API keys retain their old limits, creating a mismatch. Admins had to manually edit each key individually.
Additionally, when creating a new user, the default key did not inherit the user's configured limits -- it was created with minimal defaults regardless of the user's settings.
A secondary issue: cache invalidation failures during user operations were silently swallowed, making it hard to diagnose stale-cache bugs.
Solution
1. User-to-Key Config Sync
New lib module (
src/lib/users/user-key-sync.ts): a cent-level distribution algorithm that splits user-level limits across N keys:Math.floor, remainder discarded (logged in summary). If total cents < key count, cents assigned to early keys, rest getnull(no limit).Two new server actions:
syncUserConfigToKeys(userId, data): saves user edits, then updates all undeleted keys in a transaction. Available via "Sync to Keys" button in the edit user dialog.batchSyncUserConfigToKeys({ userIds, updates }): batch variant for the dashboard batch-edit flow. Optionally applies user-field edits first, then syncs keys per user. Capped at 500 users.2. Default Key Inherits User Settings on Creation
When a new user is created (
addUserserver action, and theCreateUserDialog), the default key now receives limits derived frombuildFirstSyncedKeyConfig()instead of bare defaults. The create-user dialog also hides the provider-group and enable-status fields from the key section (they are now controlled at the user level).3. Cache Invalidation Failure Logging
Two new helper functions (
warnUserCacheInvalidationFailure,warnKeyCacheInvalidationFailure) log warnings instead of silently swallowing cache invalidation errors during user operations.4. Settings Page: Billing Correction Section
Reorganizes the system settings form: groups the billing model source, Codex Priority billing source, and billing header rectifier toggle into a visually distinct "Billing and Rate Correction" card. Adds a direct-link button in the settings page header for quick navigation.
Changes
Core Logic
src/lib/users/user-key-sync.ts(+185) -- New: distribution algorithm,buildSyncedKeyConfigs,buildFirstSyncedKeyConfigsrc/actions/users.ts(+520/-4) -- New:syncUserConfigToKeys,batchSyncUserConfigToKeys,buildUserDbUpdates, cache warn helpers; modifiedaddUserto apply synced config to default keyUI
edit-user-dialog.tsx(+56/-2) -- "Sync to Keys" button in footerbatch-edit-dialog.tsx(+84/-34) -- Sync toggle in batch user section, updated submit logicbatch-user-section.tsx(+14) -- NewsyncKeysEnabledtoggle fieldcreate-user-dialog.tsx(+25/-11) -- UsesbuildFirstSyncedKeyConfigfor first key; hides provider-group/enable-status in key sectionkey-edit-section.tsx(+42/-34) -- NewshowProviderGroup,showEnableStatuspropssystem-settings-form.tsx(+82/-68) -- Billing correction grouped cardsettings/config/page.tsx(+7) -- Direct link to billing correction sectioni18n (5 languages)
messages/{en,ja,ru,zh-CN,zh-TW}/dashboard.json-- Sync-to-keys strings, batch dialog labels, toast messagesmessages/{en,ja,ru,zh-CN,zh-TW}/settings/config.json-- Billing correction section title/descriptionTests
tests/unit/lib/users/user-key-sync.test.ts(+66) -- New: distribution algorithm (averaging, small-amount handling, null/non-positive clearing)tests/unit/actions/users-key-sync.test.ts(+315) -- New: action-level tests for single sync, batch sync, permission checks, default key inheritancetests/unit/user-dialogs.test.tsx(+23/-3) -- Updated: "Sync to Keys" button rendering, create dialog prop assertionstests/unit/actions/users-reset-all-statistics.test.ts(+1) -- Mock fix for newinvalidateCachedKeyimportIntentionally Excluded
Temporary key groups, self-service quota windows, billing multiplier correction, status aliases, and deployment scripts -- these remain in #1106.
Testing
Automated
syncUserConfigToKeysaction (single user, permission check)batchSyncUserConfigToKeysaction (multi-user, user updates + key sync, permission check)addUserdefault key inheritanceVerification Commands
Greptile Summary
This PR implements user-to-key configuration sync: a new distribution algorithm splits user-level limits (quotas, spend caps, concurrency, provider group, reset modes) across a user's existing keys, exposed via a "Sync to Keys" button in the edit dialog and a batch sync toggle. New key creation is also updated to inherit user limits via
buildFirstSyncedKeyConfig.user-key-sync.tslines 137–140):limit5hResetModedefaults to\"rolling\"whiledailyResetModedefaults to\"fixed\"when the source value isnull/absent. If both fields are intended to share the same fallback, one of the two conditional expressions is inverted and will silently apply the wrong mode to synced keys.Confidence Score: 3/5
Functional but has a reset-mode default asymmetry that will silently apply the wrong daily reset mode to synced keys for users who have never explicitly set it.
One P1 finding (asymmetric reset-mode defaults) plus several P1s from prior threads (hardcoded i18n strings, N+1 in transaction, concurrent-session remainder not distributed, implicit unsaved-changes save on sync) pull the ceiling to 4 with multiple P1s warranting 3.
src/lib/users/user-key-sync.ts (reset mode defaults), src/actions/users.ts (hardcoded strings, N+1 loops)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Admin participant EditDialog participant SyncAction as syncUserConfigToKeys participant DB participant Cache Admin->>EditDialog: Click "Sync to Keys" EditDialog->>SyncAction: syncUserConfigToKeys(userId, formData) SyncAction->>DB: BEGIN TRANSACTION SyncAction->>DB: UPDATE users SET ... WHERE id=userId DB-->>SyncAction: updatedUser (with new limits) SyncAction->>DB: SELECT keys WHERE userId=userId DB-->>SyncAction: keyRows[] loop For each key (N+1 queries) SyncAction->>DB: UPDATE keys SET config[i] WHERE id=keyRow.id end SyncAction->>DB: COMMIT SyncAction->>Cache: invalidateCachedUser + invalidateCachedKey (fire-and-forget) SyncAction-->>EditDialog: { ok: true, keyCount } EditDialog-->>Admin: toast.success("Synced N keys")Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "feat(users): sync user limits to keys" | Re-trigger Greptile