fix(ui): improve mobile provider form UX and add client restriction config#822
Merged
fix(ui): improve mobile provider form UX and add client restriction config#822
Conversation
* fix(ui): 修复服务商表单小屏底部导航遮挡提交按钮 * test(vitest): 修复 node 内置模块 mock 并限制默认 worker 数 * fix(ui): 补齐 safe-area-bottom 并修正表单进度条 * test(e2e): 通过登录换取会话 token * test(e2e): 登录获取会话 token 增加重试 * fix(redis): CI 环境允许连接 Redis * fix(ui): 移动端 dvh 视口高度与安全区适配 * fix(e2e): 登录重试仅覆盖可重试错误 * fix(redis): 连接终止后重置单例避免僵尸客户端 * fix(ui,redis): safe-area 作用域与 Redis 配置复用 * fix(ui,redis): dvh 自适应与 closeRedis 守卫 * fix(ui,i18n,redis): 修复 max-h/i18n 与 closeRedis * fix(ui,test,a11y): 补齐小屏体验与单测稳定性 * fix(ui,test): 处理 CodeRabbit 复审建议 * fix(ui): 客户端限制输入补齐错误反馈 * fix(ui): 改善服务商表单的客户端限制与模型选择体验 * perf(ui): Provider 表单减少无效重算 * chore: format code (fix-issue-799-mobile-ui-38fb971) * fix(build): standalone 本地运行补齐静态资源 * fix(ui): 调整思考预算/自适应思考提示触发与定位 * 修复 Provider 覆盖项下拉宽度与提示交互 * 修复 TagInput 选择建议后下拉关闭 * 修复下拉信息图标阻挡点击 * 修复表单步骤进度条动画 * test(e2e): 修复登录 Cookie 解析与重试策略 * fix: 优化客户端限制开关与 E2E 重试判定 * test(e2e): 收敛 fetch retry 的错误类型 --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
📝 WalkthroughWalkthrough添加/扩展多语言翻译键、引入可配置的视口高度 CSS 变量并将大量固定 vh/100vh 替换为这些变量，新增客户端限制编辑器组件并整合到提供者表单，重构 Redis 客户端选项与生命周期管理，增加 E2E 测试登录辅助工具及若干测试更新，且伴随多处样式/可访问性与小型 UI 调整。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
Contributor
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! 此拉取请求主要致力于提升用户界面的响应性和功能性，特别是在移动设备上的表现。通过引入客户端限制编辑器，增强了服务的安全性与可配置性。同时，对后端 Redis 连接的稳定性和 E2E 测试的可靠性也进行了关键性修复，并全面完善了多语言支持和多处 UI 交互细节，以提供更流畅、更可靠的用户体验。 Highlights
Changelog
Activity
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
|
ding113
changed the title
github-actions
bot
added
size/XL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
这是一次高质量的合并请求，包含了多项重要的用户体验改进、功能增强和稳定性修复。
主要的亮点包括：
- 移动端表单体验优化：通过适配
dvh视口单位和safe-area安全区，彻底解决了小屏幕设备上底部导航遮挡提交按钮的问题。相关的 CSS 变量和布局调整实现得非常出色。 - 客户端限制功能重构：新的客户端限制编辑器组件 (
ClientRestrictionsEditor) 极大地改善了用户体验，用统一的TagInput替代了原有的复选框和输入框组合，逻辑更清晰，交互更流畅。 - 后端稳定性修复：对 Redis 单例客户端的管理进行了重构，有效解决了连接终止后的“僵尸客户端”问题，增强了系统的健壮性。
- 测试套件稳定性：通过引入新的 E2E 测试登录辅助函数，增加了重试逻辑并修复了 Cookie 解析问题，显著提高了测试的稳定性。
- UI 细节与国际化：补充了多语言 i18n key，并修复了
TagInput、下拉组件等多个 UI 交互细节，体现了对细节的关注。
代码结构清晰，重构合理，整体改动考虑周全。这是一次非常出色的工作。
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/app/[locale]/settings/providers/_components/scheduling-rules-dialog.tsx (1)
36-36:⚠️ Potential issue | 🟡 MinorEmoji 字面量违反编码规范
useScenarios()中的emoji字段（第 36、69、94、128 行）直接使用了 emoji 字符作为字符串字面量，违反了编码规范。建议将emoji字段替换为可映射到 lucide-react 图标组件的标识符，或改用纯文本/图标组件。- { title: ..., emoji: "🎯", ... } + { title: ..., icon: Target, ... } // 使用 lucide-react 中的 Target 图标并相应修改
ScenarioCardProps.emoji: string为icon: React.ComponentType<...>。As per coding guidelines:
**/*.{js,ts,tsx,jsx}: Never use emoji characters in any code, comments, or string literals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/scheduling-rules-dialog.tsx at line 36, Replace the emoji string literals in useScenarios() with icon identifiers or React components and update the prop type accordingly: change each scenario object field named emoji to icon (a React component type) and update ScenarioCardProps.emoji: string to ScenarioCardProps.icon: React.ComponentType<React.SVGProps<SVGSVGElement>> (or an appropriate component prop type used in your codebase); then update all consumers (e.g., ScenarioCard and any render sites) to render the icon component instead of using the emoji string; ensure all four occurrences in useScenarios() are replaced and type imports/exports are adjusted.src/app/[locale]/internal/dashboard/big-screen/page.tsx (3)
818-819:⚠️ Potential issue | 🟡 Minor按钮
title属性存在硬编码中文及疑似乱码
title属性值当前: ... (点击切换)应走 i18n，且切��末尾字符看起来已损坏（可能是编码截断）。请检查源文件编码并将文案提取为翻译 key。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/internal/dashboard/big-screen/page.tsx around lines 818 - 819, The title prop currently contains hardcoded Chinese with a corrupted tail ("点击切��") — replace it with an i18n string and remove the hardcoded text; use the existing localeLabels and currentLocale to build a translated label (e.g. t('bigScreen.currentLocale', { locale: localeLabels[currentLocale] })) and set title={t(...)} on the element that uses localeLabels/currentLocale, and also fix the source file encoding so the original Chinese text is not corrupted before extracting the translation key.
609-609:⚠️ Potential issue | 🟡 Minor硬编码中文字符串违反 i18n 规范
formatter回调中直接写死了"请求"和"数量"两个用户可见字符串，未走t()翻译。- formatter={(value) => [`${value ?? 0} 请求`, "数量"]} + formatter={(value) => [`${value ?? 0} ${t("chart.requests")}`, t("chart.quantity")]}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/internal/dashboard/big-screen/page.tsx at line 609, The formatter prop currently hardcodes user-visible strings ("请求" and "数量") in the arrow function (formatter={(value) => [`${value ?? 0} 请求`, "数量"]}); replace those literals with calls to the i18n translation function (e.g. t('requests') and t('count')) and ensure t is available in the component scope by using the existing i18n hook (useTranslation or the app's t provider)—update the formatter to formatter={(value) => [`${value ?? 0} ${t('requests')}`, t('count')]} and add the necessary import/useTranslation usage so translations are used instead of hardcoded Chinese.
656-656:⚠️ Potential issue | 🟡 Minor"暂无数据"硬编码违反 i18n 规范
ModelDistribution空状态文案未使用翻译函数。- <span className={`text-xs ${theme.text} opacity-50`}>暂无数据</span> + <span className={`text-xs ${theme.text} opacity-50`}>{t("sections.noData")}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/internal/dashboard/big-screen/page.tsx at line 656, The empty-state string "暂无数据" in the ModelDistribution component is hardcoded and must be localized; replace that literal in the JSX span with a call to the project's translation function (e.g., the useTranslations/useTranslation hook or i18n.t) and pass an appropriate key (for example "dashboard.noData" or similar), import/obtain the translation function in the ModelDistribution scope if not already present, and use the translated value inside the span that currently uses theme.text to render the localized empty-state label.src/app/global-error.tsx (1)
56-115:⚠️ Potential issue | 🟡 Minor组件内所有用户可见文本均为硬编码英文，违反 i18n 规范
global-error.tsx中的所有面向用户的文案（如"Network Connection Error"、"Try again"、"Go to Home"等）均未走多语言翻译，违反了"所有用户可见字符串必须使用 i18n"的编码规范。已知技术限制：该组件在根 Layout 抛出错误时渲染，此时
next-intl的Provider不在组件树中，因此无法使用useTranslations。如需支持 i18n，需手动读取 locale cookie 并动态加载对应翻译文件。建议至少在文件头部添加注释说明这一限制，或在后续 sprint 中通过 cookie + 动态 import 实现该边界组件的多语言支持。
As per coding guidelines: All user-facing strings must use i18n (5 languages supported). Never hardcode display text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/global-error.tsx` around lines 56 - 115, The component global-error.tsx hardcodes all user-facing strings (e.g. "Network Connection Error", "Try again", "Go to Home", the paragraph and list items) which breaks the i18n policy; either add a clear top-of-file comment explaining next-intl Provider is unavailable during root layout errors and that these strings are intentionally not translated yet, or implement proper i18n by reading the locale cookie and dynamically importing the translation bundle and replacing hardcoded literals with lookups (ensure texts used in render paths around reset() and handleGoHome are replaced with translation keys); include the 5 supported languages and a TODO referencing a follow-up sprint if you opt for the comment approach.
♻️ Duplicate comments (3)
src/app/[locale]/settings/prices/_components/sync-conflict-dialog.tsx (1)
254-254: CSS 变量回退值问题与create-user-dialog.tsx相同。
max-h-[var(--cch-viewport-height-80)]缺少回退值，建议改为var(--cch-viewport-height-80,80vh)，详见create-user-dialog.tsx中的相关说明。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/prices/_components/sync-conflict-dialog.tsx at line 254, In the SyncConflictDialog's JSX (the DialogContent element) the Tailwind class uses max-h-[var(--cch-viewport-height-80)] without a CSS variable fallback; update that class to include a fallback (e.g., change max-h-[var(--cch-viewport-height-80)] to max-h-[var(--cch-viewport-height-80,80vh)]) so the dialog has a sensible height when the CSS variable is undefined—locate the DialogContent element in sync-conflict-dialog.tsx and make this replacement to match the fix used in create-user-dialog.tsx.src/app/[locale]/settings/providers/_components/add-provider-dialog.tsx (1)
25-25: CSS 变量回退值问题同上，建议一并修复。
max-h-[var(--cch-viewport-height-90)]与create-user-dialog.tsx存在同样的缺少回退值问题，建议改为var(--cch-viewport-height-90,90vh)。新增的响应式宽度约束（max-w-full sm:max-w-5xl lg:max-w-6xl）和overflow-hidden是合理的 UX 改进。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/add-provider-dialog.tsx at line 25, The class on DialogContent uses a CSS variable without a fallback (max-h-[var(--cch-viewport-height-90)]) — update the value to include a sensible fallback, e.g., replace the variable usage in the DialogContent class string so max-h uses var(--cch-viewport-height-90,90vh); leave the new responsive width constraints (max-w-full sm:max-w-5xl lg:max-w-6xl) and overflow-hidden intact; locate the DialogContent component in add-provider-dialog.tsx and modify its className accordingly.src/app/[locale]/dashboard/_components/dashboard-main.tsx (1)
23-27: CSS 变量高度替换 LGTM；嵌套<main>问题请参见layout.tsx中的评论此处新增的
<main>包裹标签与根布局 (layout.tsx第 84 行) 中已有的<main>形成嵌套。已在layout.tsx中标注了根本原因，建议统一处理。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/dashboard-main.tsx around lines 23 - 27, The DashboardMain component currently renders a <main> that nests inside the root layout's <main>; replace the inner <main> in DashboardMain with a non-landmark element (e.g., a <div>) that preserves the existing className ("h-[calc(var(--cch-viewport-height,100vh)-64px)] w-full overflow-hidden") and children, or alternatively move the height/class styling to the root layout component so only one <main> exists; update the DashboardMain render (the component currently returning the inner <main>) accordingly to avoid nested landmark elements and preserve layout and accessibility.
🧹 Nitpick comments (23)
tests/unit/settings/providers/thinking-budget-editor.test.tsx (1)
117-119: 可考虑抽一个查找 max-out 按钮的辅助函数，减少重复。建议的精简方式
+const findMaxOutButton = (container: HTMLElement) => + Array.from(container.querySelectorAll("button")).find((b) => + b.textContent?.includes("maxOutButton") + ); ... - const maxButton = Array.from(container.querySelectorAll("button")).find((b) => - b.textContent?.includes("maxOutButton") - ); + const maxButton = findMaxOutButton(container); ... - const maxButton = Array.from(container.querySelectorAll("button")).find((b) => - b.textContent?.includes("maxOutButton") - ) as HTMLButtonElement; + const maxButton = findMaxOutButton(container) as HTMLButtonElement; ... - const maxButton = Array.from(container.querySelectorAll("button")).find((b) => - b.textContent?.includes("maxOutButton") - ) as HTMLButtonElement; + const maxButton = findMaxOutButton(container) as HTMLButtonElement;Also applies to: 168-170, 237-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/settings/providers/thinking-budget-editor.test.tsx` around lines 117 - 119, Extract a small test helper like findMaxOutButton(container) (or getMaxOutButton) that encapsulates the current logic Array.from(container.querySelectorAll("button")).find(b => b.textContent?.includes("maxOutButton")), then replace the three inline occurrences that assign maxButton with calls to that helper in the thinking-budget-editor.test.tsx file so tests use a single reusable function for locating the max-out button.vitest.config.ts (1)
8-12:parsePositiveInt不支持 Vitest 原生的百分比字符串（如'50%'）Vitest 的
maxWorkers同时支持数字和百分比字符串（例如maxWorkers: '50%'）。当前parsePositiveInt遇到VITEST_MAX_WORKERS='50%'时，Number.parseInt('50%', 10)会返回50（因为parseInt会解析前导数字部分），实际上会静默地将'50%'当作整数50，而非百分比。如需百分比语义，可考虑保留字符串传透：♻️ 可选：支持百分比字符串透传
-function parsePositiveInt(value: string | undefined, fallback: number): number { +function parseMaxWorkers(value: string | undefined, fallback: number): number | string { if (!value) return fallback; + if (/^\d+(\.\d+)?%$/.test(value)) return value; const parsed = Number.parseInt(value, 10); return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vitest.config.ts` around lines 8 - 12, parsePositiveInt currently uses Number.parseInt which will coerce inputs like '50%' to 50 and lose percentage semantics required by Vitest's maxWorkers; update parsePositiveInt (and any callers that set maxWorkers from VITEST_MAX_WORKERS) to detect strings ending with '%' and return the original string unchanged, otherwise validate and return a positive integer fallback if invalid—ensure the function signature supports returning string | number (or adjust the flow where maxWorkers is consumed to accept both types) and reference parsePositiveInt and the VITEST_MAX_WORKERS / maxWorkers usage to locate the change.src/app/[locale]/settings/providers/_components/model-multi-select.tsx (1)
43-73:loading属性是死代码，建议移除以简化组件接口
ModelSourceIndicator仅在父组件的{!loading && (...)}分支中渲染（第 245 行），因此传入的loading永远为false，第 54 行的if (loading) return null;永远不会执行。♻️ 建议移除冗余的 `loading` 属性
function ModelSourceIndicator({ - loading, isUpstream, label, description, }: { - loading: boolean; isUpstream: boolean; label: string; description: string; }) { - if (loading) return null; - const Icon = isUpstream ? Cloud : Database;同步更新调用处（第 251-256 行）：
<ModelSourceIndicator - loading={loading} isUpstream={isUpstream} label={sourceLabel} description={sourceDescription} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/model-multi-select.tsx around lines 43 - 73, The ModelSourceIndicator component has an unused/dead prop loading (the early return if (loading) return null is never hit because the parent only renders the component when !loading); remove the loading prop and the if (loading) check from the ModelSourceIndicator definition, update its props type to only accept isUpstream, label, description, and update all call sites that pass loading (the parent render where ModelSourceIndicator is invoked) to stop passing loading so props align; ensure Tooltip/TooltipTrigger/TooltipContent usage remains unchanged.src/components/ui/tag-input.tsx (1)
157-175: LGTM — capture-phase 修复方案正确
isInContainer/isInDropdown的拆分解决了 Portal 渲染的下拉列表不在containerRefDOM 树内、导致外部点击被误判的根本问题；改用 capture 阶段监听也正确规避了 React 在mousedown后同步 reconcile 导致e.target被卸载的问题。可选优化：当前解释 capture 阶段原因的注释（lines 166-167）位于 handler 内部，实际上描述的是 为什么注册为 capture，放在
addEventListener调用处（line 174）可读性更佳。♻️ 建议的注释迁移
- // 使用 capture 阶段监听，避免点击建议项后 React 同步更新导致 target 节点被移除， - // 进而被误判为"点击了外部"并关闭下拉列表。 if (!isInContainer && !isInDropdown) { setShowSuggestions(false); setHighlightedIndex(-1); } }; + // 使用 capture 阶段监听，避免点击建议项后 React 同步更新导致 target 节点被移除， + // 进而被误判为"点击了外部"并关闭下拉列表。 document.addEventListener("mousedown", handleClickOutside, true); return () => document.removeEventListener("mousedown", handleClickOutside, true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/tag-input.tsx` around lines 157 - 175, Minor readability tweak: move the comment explaining why the listener uses capture phase out of the handleClickOutside body and place it immediately above the document.addEventListener("mousedown", handleClickOutside, true) call so it's clear the comment describes the reason for passing true (capture). In practice, edit the component around the useEffect that defines handleClickOutside (references: containerRef, dropdownRef, handleClickOutside, setShowSuggestions, setHighlightedIndex) — remove or shorten the capture explanation inside the handler and add a concise comment above the addEventListener line describing that capture phase is used to avoid React's synchronous unmount/reconcile removing e.target before the handler runs.src/lib/redis/client.ts (1)
156-173: closeRedis 可考虑先“标记关闭”以避免并发重用如果 closeRedis 在仍有并发请求时被调用，当前逻辑在
quit()期间仍可能被 getRedisClient 复用。可选地提前清空redisClient，避免返回一个正在关闭的实例。建议修改
export async function closeRedis(): Promise<void> { const client = redisClient; if (!client) return; + if (redisClient === client) { + redisClient = null; + } try { if (client.status !== "end") { await client.quit(); } } catch (error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/redis/client.ts` around lines 156 - 173, closeRedis can race with getRedisClient and return a client that's in the process of quitting; to avoid concurrent reuse, atomically mark the global redisClient as null at the start of closeRedis (store the current instance in a local variable, set redisClient = null) before calling client.quit(), then proceed with the existing try/catch (using client.quit() and client.disconnect() on error) and only clear redisClient in finally if it still equals the captured instance; update references to redisClient, closeRedis, getRedisClient and the client.quit()/client.disconnect() flow accordingly.tests/unit/lib/endpoint-circuit-breaker.test.ts (1)
28-33: 建议用vi.waitFor()替换手写轮询，并修复超时无声返回的问题当前实现存在以下问题：
超时静默返回：若
timeoutMs耗尽仍无调用，函数直接返回而不抛出异常。当 mock 永不被调用（生产代码 bug）时，测试会挂起 5 秒，随后以不清晰的断言错误失败，而非一条描述性的"等待 mock 调用超时"错误信息。假计时器风险：
setTimeout(resolve, 0)在vi.useFakeTimers()激活时无法触发，会导致函数无限挂起。当前测试在afterEach中始终调用vi.useRealTimers()进行防护，但 helper 本身缺乏文档说明和内部防护，存在日后重构时遗漏的隐患。超时时间过长：5000ms 对单元测试不适宜。若断言真的失败，每次都需等待 5 秒才能反馈问题。
Vitest 内置的
vi.waitFor(callback, options?)自 v0.34.5 起可用（当前项目使用 v4.0.16）。该 API 持续重试回调直至成功或超时，超时时自动抛出描述性错误。更重要的是，它会自动处理 fake timer 的推进，无需手动干预。建议直接替换为：await vi.waitFor(() => expect(sendAlertMock).toHaveBeenCalledTimes(1), { timeout: 1000 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/lib/endpoint-circuit-breaker.test.ts` around lines 28 - 33, The helper waitForMockCalled currently polls with setTimeout(0) causing silent timeouts, fake-timer hangs, and an overly long default timeout; replace its implementation to use vitest's vi.waitFor (which advances fake timers and throws on timeout) and reduce the default timeout (e.g., 1000ms); update references to waitForMockCalled to call await vi.waitFor(() => expect(mock).toHaveBeenCalled(), { timeout: 1000 }) or equivalent so failures raise descriptive errors rather than silently returning.src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx (1)
152-152: 同：CSS 变量缺少回退值与前述两个对话框相同，建议补充
var(--cch-viewport-height-70,70dvh)的回退值。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx at line 152, 在 edit-key-quota-dialog.tsx 中的 DialogContent 组件的 className 使用了 CSS 变量 --cch-viewport-height-70 但没有回退值；请将 `max-h-[var(--cch-viewport-height-70)]` 更新为带回退值的形式（例如 `var(--cch-viewport-height-70,70dvh)`）以确保在变量未定义时仍有合理高度回退，定位修改点为 DialogContent 的 className 字符串。src/app/[locale]/settings/providers/_components/forms/test-result-card.tsx (1)
175-175: 建议：CSS 变量缺少回退值，与布局文件存在不一致
settings/layout.tsx采用var(--cch-viewport-height,100vh)并提供了显式回退值，而此处的对话框及其他同类对话框（edit-user-quota-dialog.tsx、edit-key-quota-dialog.tsx）均未提供回退值。若 CSS 变量未加载（如 SSR hydration 瞬态、样式文件加载失败），对话框将丢失max-h约束。建议添加回退值以保持一致性：
✨ 建议修复
-<DialogContent className="max-w-4xl max-h-[var(--cch-viewport-height-85)] overflow-y-auto"> +<DialogContent className="max-w-4xl max-h-[var(--cch-viewport-height-85,85dvh)] overflow-y-auto">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/test-result-card.tsx at line 175, The DialogContent in test-result-card.tsx uses CSS var(--cch-viewport-height) without a fallback, so add an explicit fallback matching settings/layout.tsx (e.g. var(--cch-viewport-height, 100vh)) to the max-h declaration to prevent losing the height constraint if the variable isn't available; apply the same fix to the other dialog components mentioned (edit-user-quota-dialog.tsx and edit-key-quota-dialog.tsx) where DialogContent or similar uses var(--cch-viewport-height) so all dialog max-h usages remain consistent and resilient.src/app/[locale]/dashboard/quotas/keys/_components/edit-user-quota-dialog.tsx (1)
90-90: 同：CSS 变量缺少回退值与
test-result-card.tsx中的情况相同，建议补充回退值var(--cch-viewport-height-70,70dvh)以与settings/layout.tsx的防御性写法保持一致。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/quotas/keys/_components/edit-user-quota-dialog.tsx at line 90, 在 edit-user-quota-dialog.tsx 的 DialogContent 元素中使用的 CSS 变量缺少回退值；将类名中出现的 max-h 样式从 var(--cch-viewport-height-70) 改为带回退值的 var(--cch-viewport-height-70,70dvh)（在组件 EditUserQuotaDialog 或使用 DialogContent 的位置更新该字符串），保证在变量未定义时仍使用 70dvh 作为后备高度。src/app/[locale]/dashboard/_components/user/create-user-dialog.tsx (1)
340-340: 建议为 CSS 变量添加回退值以增强健壮性
max-h-[var(--cch-viewport-height-90)]没有显式回退值。若--cch-viewport-height-90未定义（例如globals.css加载失败或 SSR 场景），max-height会降级为初始值none，导致对话框溢出视口。对比同 PR 中
session-details-tabs.tsx的写法var(--cch-viewport-height, 100vh)，其带有明确的100vh回退。建议各对话框组件保持一致：♻️ 建议添加 CSS 回退值（同适用于 sync-conflict-dialog.tsx 与 add-provider-dialog.tsx）
- <DialogContent className="w-full max-w-[95vw] sm:max-w-[85vw] md:max-w-[70vw] lg:max-w-3xl max-h-[var(--cch-viewport-height-90)] p-0 flex flex-col overflow-hidden"> + <DialogContent className="w-full max-w-[95vw] sm:max-w-[85vw] md:max-w-[70vw] lg:max-w-3xl max-h-[var(--cch-viewport-height-90,90vh)] p-0 flex flex-col overflow-hidden">同理，
sync-conflict-dialog.tsx第 254 行改为var(--cch-viewport-height-80,80vh)，add-provider-dialog.tsx第 25 行改为var(--cch-viewport-height-90,90vh)。🤖 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 at line 340, The DialogContent in create-user-dialog.tsx uses max-h-[var(--cch-viewport-height-90)] without a fallback; update the CSS variable usage inside the DialogContent className (in the create-user-dialog component) to include a sensible fallback like var(--cch-viewport-height-90,90vh) so the dialog keeps a max-height if the custom property is undefined; apply the same pattern to the other dialog components mentioned (sync-conflict-dialog.tsx: replace var(--cch-viewport-height-80) with var(--cch-viewport-height-80,80vh) and add-provider-dialog.tsx: replace var(--cch-viewport-height-90) with var(--cch-viewport-height-90,90vh)).tests/unit/login/login-visual-regression.test.tsx (1)
67-71: 第 71 行断言冗余Line 67 的选择器
div.bg-gradient-to-br已通过.bg-gradient-to-br类定位元素，若mainContainer非null，其className必然包含"bg-gradient-to"。因此 Line 71 的断言永远不会失败，也无法提供额外保障。♻️ 建议移除冗余断言
const className = mainContainer?.className || ""; expect(className).toContain("min-h-[var(--cch-viewport-height,100vh)]"); - expect(className).toContain("bg-gradient-to");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/login/login-visual-regression.test.tsx` around lines 67 - 71, The assertion on Line 71 is redundant because the selector div.bg-gradient-to-br already guarantees the element has the bg-gradient-to class; remove the expect(className).toContain("bg-gradient-to") assertion from the test (the variables to locate are mainContainer and className in tests/unit/login/login-visual-regression.test.tsx) and keep the non-null check and remaining assertions (e.g., min-h[...] and any other meaningful checks).src/app/[locale]/settings/providers/_components/provider-list-item.legacy.tsx (1)
130-145: 文件内存在大量违反 i18n 规范的硬编码中文字符串及 emoji（第213、263行的🔴/🟡），与项目编码规范不符。当前文件存在以下违规（均为既有代码）：
toast.success("熔断器已重置", ...)等 toast 消息直接使用中文字面量- Badge 内含 emoji 字符（第213行
🔴、第263行🟡），违反 "Never use emoji characters in any code, comments, or string literals" 规范- 多处 aria-label、DialogTitle、Tooltip 文本未经 i18n
建议在
.legacy.tsx组件的后续清理中统一通过tList(...)等翻译函数替换，或在文件头部增加 lint 豁免注释以明确维护意图。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/provider-list-item.legacy.tsx around lines 130 - 145, This file contains hardcoded Chinese strings and emoji literals (e.g., toast.success/toast.error messages and Badge content using "🔴"/"🟡") that violate i18n and emoji policies; replace all literal UI strings in provider-list-item.legacy.tsx—including toast messages (toast.success, toast.error), Badge contents, aria-labels, DialogTitle and Tooltip text—with calls to the project's i18n helpers (e.g., tList(...) or equivalent translation function) and remove/replace emoji characters with translatable status labels or icons; if you must defer full translation, add a single-file lint/exemption comment at the top of provider-list-item.legacy.tsx documenting why and when it will be converted to i18n to satisfy lint rules.scripts/copy-version-to-standalone.cjs (1)
4-13:copyDirIfExists函数逻辑正确；fs.cpSync实验性状态不影响当前使用。
fs.cpSync自 Node.js v16.7.0 起以实验性 API（Stability: 1）引入，在 Next.js 16 所要求的 Node.js 18.17+ 中虽仍带 experimental 标记，但功能稳定且不存在实际问题。该 API 仅在此脚本中使用一次（第 11 行），且无需特殊错误处理（构建脚本失败即可）。如需使用完全稳定的 API，可改为
fs-extra或手动递归复制，但这是可选优化。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/copy-version-to-standalone.cjs` around lines 4 - 13, 函数 copyDirIfExists 使用的 fs.cpSync（在代码中第 11 行）虽带 experimental 标记但在 Node 18.17+ 中可正常使用；无需修改——保留现状即可；如果你要消除 experimental 标记，替换点为在 copyDirIfExists 中用 fs-extra 的 copySync（或手动递归复制）并添加 fs-extra 依赖，替换所有对 fs.cpSync 的调用并保持 mkdirSync(path.dirname(dstDir), { recursive: true }) 和相同的日志行为。src/app/[locale]/settings/providers/_components/batch-edit/provider-batch-preview-step.tsx (1)
173-174: 考虑为formatValue函数采用更具体的类型定义虽然项目当前未启用 next-intl 的严格类型检查，但建议参照 next-intl 最佳实践为
t参数使用专用类型，提升类型安全性。如果将来启用严格类型检查，当前的泛型签名(key: string) => string会因为 TypeScript 函数参数的逆变规则产生编译错误。建议改为：
-function formatValue(value: unknown, t: (key: string) => string): string { +function formatValue( + value: unknown, + t: ReturnType<typeof useTranslations<"settings.providers.batchEdit">>, +): string {或使用类型别名简化：
+type TranslateFn = ReturnType<typeof useTranslations<"settings.providers.batchEdit">>; + -function formatValue(value: unknown, t: (key: string) => string): string { +function formatValue(value: unknown, t: TranslateFn): string {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/batch-edit/provider-batch-preview-step.tsx around lines 173 - 174, The formatValue function currently types the translator as a generic (key: string) => string which can break under strict next-intl typing; update formatValue's t parameter to use next-intl's proper translator type (e.g. import and use TFunction or define a type alias like Translate = TFunction) so signature becomes formatValue(value: unknown, t: Translate): string and adjust any imports accordingly; keep the function name formatValue and only change the t parameter type to the next-intl-specific type to ensure compatibility with strict type checking.src/components/ui/drawer.tsx (2)
53-54:max-hCSS 变量缺少回退值，与布局文件不一致
var(--cch-viewport-height-80)/var(--cch-viewport-height-95)均未提供回退值。若变量未定义，max-height将退化为none（即无高度限制），可能导致内容溢出屏幕。对比同 PR 中的布局文件（
dashboard/layout.tsx、my-usage/layout.tsx、big-screen/loading.tsx），均使用var(--cch-viewport-height,100vh)带有100vh回退。同样的问题也影响edit-key-dialog.tsx（第 55 行）、filter-dialog.tsx（第 263 行）、session-messages-dialog.tsx（第 70 行）、batch-edit-dialog.tsx（第 446 行）。建议统一添加回退值：
建议改动
- "data-[vaul-drawer-direction=top]:max-h-[var(--cch-viewport-height-80)] ..." + "data-[vaul-drawer-direction=top]:max-h-[var(--cch-viewport-height-80,80vh)] ..." - "data-[vaul-drawer-direction=bottom]:max-h-[var(--cch-viewport-height-95)] ..." + "data-[vaul-drawer-direction=bottom]:max-h-[var(--cch-viewport-height-95,95vh)] ..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/drawer.tsx` around lines 53 - 54, The max-height CSS custom properties in src/components/ui/drawer.tsx use var(--cch-viewport-height-80) and var(--cch-viewport-height-95) without fallbacks; update those to include a fallback (e.g. var(--cch-viewport-height-80, 100vh) and var(--cch-viewport-height-95, 100vh)) so max-h never becomes none if the variable is undefined, and apply the same change to the other affected components mentioned (edit-key-dialog, filter-dialog, session-messages-dialog, batch-edit-dialog) where var(--cch-viewport-height-*) is used.
52-52: 左/右方向抽屉的横屏安全区处理不完整代码在 line 52 对
left和right方向抽屉应用了safe-area-bottom，但该类仅处理底部内缩。若抽屉在横屏刘海屏设备上使用，还需要额外处理safe-area-inset-left和safe-area-inset-right。当前
.safe-area-bottom定义为padding-bottom: var(--safe-area-pb, env(safe-area-inset-bottom, 0px))，仅适用于底部。建议为left和right方向添加对应的safe-area-left和safe-area-right工具类，分别使用env(safe-area-inset-left)和env(safe-area-inset-right)。注：当前项目实际使用的抽屉均为
bottom方向，此处理为预防性优化。若无横屏左/右抽屉的使用计划，可暂时忽略。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/drawer.tsx` at line 52, The drawer component currently applies "data-[vaul-drawer-direction=left]:safe-area-bottom" and "data-[vaul-drawer-direction=right]:safe-area-bottom" which only adds bottom padding; update the UI so left/right drawers use left/right safe-area insets: add utility classes safe-area-left (padding-left: env(safe-area-inset-left, 0px)) and safe-area-right (padding-right: env(safe-area-inset-right, 0px)) to your CSS, then change the class list in the drawer component string (in src/components/ui/drawer.tsx where the data-[vaul-drawer-direction=...] entries are defined) to use data-[vaul-drawer-direction=left]:safe-area-left and data-[vaul-drawer-direction=right]:safe-area-right while keeping bottom drawers using safe-area-bottom.src/app/[locale]/dashboard/_components/user/edit-user-dialog.tsx (1)
247-247: CSS 变量内联回退值不一致此处使用
var(--cch-viewport-height-90,90vh)，带有内联回退值90vh，而本 PR 中其他同类对话框（add-user-dialog.tsx、user-list.tsx、auto-sort-priority-dialog.tsx）的派生变量均不含内联回退。由于globals.css已为--cch-viewport-height-90提供默认值，内联回退是冗余的。建议统一去掉内联回退以保持一致性：
♻️ 建议改动
- <DialogContent className="w-full max-w-[95vw] sm:max-w-[85vw] md:max-w-[70vw] lg:max-w-3xl max-h-[var(--cch-viewport-height-90,90vh)] p-0 flex flex-col overflow-hidden"> + <DialogContent className="w-full max-w-[95vw] sm:max-w-[85vw] md:max-w-[70vw] lg:max-w-3xl max-h-[var(--cch-viewport-height-90)] p-0 flex flex-col overflow-hidden">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/edit-user-dialog.tsx at line 247, 在 edit-user-dialog.tsx 的 DialogContent 元素中使用了带内联回退值的 CSS 变量 var(--cch-viewport-height-90,90vh)，但项目中其它对话框（如 add-user-dialog.tsx、user-list.tsx、auto-sort-priority-dialog.tsx）均不带内联回退且 globals.css 已提供默认值；请在文件中的 DialogContent className（包含 max-h-[var(--cch-viewport-height-90,90vh)]）中移除内联回退，改为使用一致的 var(--cch-viewport-height-90) 形式以保持风格一致并依赖 globals.css 的默认值。src/components/form/client-restrictions-editor.tsx (1)
91-99:getPresetLabel作为useMemo依赖项要求调用方使用useCallback
getPresetLabel是函数类型的 prop，若调用方未将其包装在useCallback中，每次父组件渲染都会产生新的函数引用，导致suggestions在每次渲染时都被重新计算，失去 memoization 的收益。建议在
ClientRestrictionsEditorProps中补充文档注释提示调用方：♻️ 可选：添加注释说明稳定引用要求
+ /** + * 将 preset value 转换为显示标签的函数。 + * 调用方应用 useCallback 包装，以避免 suggestions 在每次渲染时重新计算。 + */ getPresetLabel: (presetValue: string) => string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/form/client-restrictions-editor.tsx` around lines 91 - 99, The prop getPresetLabel (used inside the useMemo that computes suggestions) requires a stable reference to preserve memoization; update the ClientRestrictionsEditorProps definition to add a JSDoc comment on getPresetLabel explaining callers should pass a memoized function (e.g., via React.useCallback) so that suggestions (computed from CLIENT_RESTRICTION_PRESET_OPTIONS into TagInputSuggestion[]) remains stable; mention getPresetLabel, suggestions, useMemo and the expected function signature in the comment to make the requirement discoverable to consumers.src/app/[locale]/usage-doc/layout.tsx (1)
45-45: 内联100vh降级冗余但无害
--cch-viewport-height在:root中始终有定义，var(--cch-viewport-height,100vh)的内联降级实际不会触发。保留此写法作为防御性编码可接受，但其他使用var(--cch-viewport-height-*)的组件均未添加内联降级，风格略有不一致。♻️ 可选：与其他组件风格统一
- <div className="min-h-[var(--cch-viewport-height,100vh)] bg-background"> + <div className="min-h-[var(--cch-viewport-height)] bg-background">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/usage-doc/layout.tsx at line 45, 当前布局中的内联降级值“100vh”在 div with class "min-h-[var(--cch-viewport-height,100vh)] bg-background" 中是多余的且与项目中其他使用 var(--cch-viewport-height-*) 的组件风格不一致；为统一风格，删除该内联降级（改为 "min-h-[var(--cch-viewport-height)] bg-background"）或者如果你想保留防御性降级，则在其它使用 var(--cch-viewport-height-*) 的组件也添加相同的 ",100vh" 降级以保持一致性；修改时定位到该 div（layout.tsx）并更新类字符串以实施一致方案。src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx (1)
288-297: 提取常量并消除onInvalidTag处理的重复代码。Line 292 的
{ max: 64 }虽然与ClientRestrictionsEditor内部的maxTagLength配置一致，但缺少对应的命名常量。建议按照GROUP_TAG_MAX_TOTAL_LENGTH的模式提取为常量，保证代码清晰和后续维护。同时，
onInvalidTag的 messages map 处理在本文件的 lines 168-177（group tags）和 lines 288-297（client restrictions）中重复，可以提取为工具函数：示例改进方案
const GROUP_TAG_MAX_TOTAL_LENGTH = 50; +const CLIENT_TAG_MAX_LENGTH = 64; + +function buildInvalidTagMessage( + tUI: (key: string, values?: Record<string, unknown>) => string, + reason: string, + maxLength: number, +): string { + const messages: Record<string, string> = { + empty: tUI("emptyTag"), + duplicate: tUI("duplicateTag"), + too_long: tUI("tooLong", { max: maxLength }), + invalid_format: tUI("invalidFormat"), + max_tags: tUI("maxTags"), + }; + return messages[reason] || reason; +}然后在两处
onInvalidTag回调中调用：- onInvalidTag={(_tag, reason) => { - const messages: Record<string, string> = { - empty: tUI("emptyTag"), - duplicate: tUI("duplicateTag"), - too_long: tUI("tooLong", { max: 64 }), - invalid_format: tUI("invalidFormat"), - max_tags: tUI("maxTags"), - }; - toast.error(messages[reason] || reason); - }} + onInvalidTag={(_tag, reason) => { + toast.error(buildInvalidTagMessage(tUI, reason, CLIENT_TAG_MAX_LENGTH)); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx around lines 288 - 297, Extract the tag length constant (e.g. GROUP_TAG_MAX_TOTAL_LENGTH style) for the max tag length used in ClientRestrictionsEditor and replace the hardcoded `{ max: 64 }` with that constant; then factor the duplicated onInvalidTag messages map into a single utility function (e.g. createInvalidTagHandler or getInvalidTagMessage) and call it from both places where onInvalidTag is used (the group tags handler around lines 168-177 and the client restrictions handler around lines 288-297), ensuring the utility accepts the translation function `tUI` and the max length constant so both handlers use the same source of truth.tests/e2e/_helpers/auth.ts (2)
114-148:retryCodesSet 每次调用都会重新分配，建议提取为模块级常量。♻️ 建议修改
+const RETRY_ERROR_CODES = new Set([ + "ECONNREFUSED", + "ECONNRESET", + "ETIMEDOUT", + "EAI_AGAIN", + "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", + "UND_ERR_BODY_TIMEOUT", + "UND_ERR_SOCKET", +]); + function shouldRetryFetchError(error: unknown): boolean { if (!(error instanceof Error)) return false; - const retryCodes = new Set([ - "ECONNREFUSED", - "ECONNRESET", - "ETIMEDOUT", - "EAI_AGAIN", - "UND_ERR_CONNECT_TIMEOUT", - "UND_ERR_HEADERS_TIMEOUT", - "UND_ERR_BODY_TIMEOUT", - "UND_ERR_SOCKET", - ]); - const errorWithCause = error as { cause?: unknown; code?: unknown }; const maybeCodes: string[] = []; ... if (maybeCodes.length > 0) { - return maybeCodes.some((code) => retryCodes.has(code)); + return maybeCodes.some((code) => RETRY_ERROR_CODES.has(code)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/_helpers/auth.ts` around lines 114 - 148, The retryCodes Set in shouldRetryFetchError is recreated on every call; extract it to a module-level constant (e.g., RETRY_ERROR_CODES) and replace the local retryCodes usage in shouldRetryFetchError with that constant; ensure the exported/declared constant contains the same string entries and keep the rest of the function logic unchanged (refer to shouldRetryFetchError and the current retryCodes identifier to locate where to change).
157-215: 退避时间计算在两处重复（行 174 和 201），可提取为内联辅助函数。♻️ 建议修改
+ const backoffMs = (attempt: number) => Math.min(1000, 100 * 2 ** (attempt - 1)); + for (let attempt = 1; attempt <= maxAttempts; attempt++) { ... } catch (error) { ... - await sleep(Math.min(1000, 100 * 2 ** (attempt - 1))); + await sleep(backoffMs(attempt)); continue; } ... - await sleep(Math.min(1000, 100 * 2 ** (attempt - 1))); + await sleep(backoffMs(attempt)); continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/_helpers/auth.ts` around lines 157 - 215, The retry backoff calculation (Math.min(1000, 100 * 2 ** (attempt - 1))) is duplicated in the POST loop; create a small inline helper like computeBackoffDelay(attempt) inside the same scope and replace both occurrences with calls to that helper so both the catch branch (where shouldRetryFetchError is used) and the 503 SESSION_CREATE_FAILED branch use the same computed delay before awaiting sleep; keep references to maxAttempts, attempt, sleep and shouldRetryFetchError unchanged.tests/e2e/users-keys-complete.test.ts (1)
37-37: 两处可读性改善：模块级 token 变量命名不统一，beforeAll声明顺序不符合惯例。
命名不一致：同目录的
api-complete.test.ts和notification-settings.test.ts均用authToken命名模块级 token，而本文件使用sessionToken（L37）。此外，callApi的第四个参数（L65）局部名也叫authToken，在同一文件中混用两个名称指向同一概念，容易误读。
beforeAll在afterAll之后：新增的beforeAll（L192–195）声明于afterAll（L171）之后，违反通常将设置钩子写在清理钩子之前的约定。♻️ 建议修改（命名统一 + 调整声明顺序）
-let sessionToken: string | undefined; +let authToken: string | undefined;async function callApi( module: string, action: string, body: Record<string, unknown> = {}, - authToken = sessionToken, + authToken_ = authToken, ) { - if (!authToken) { + if (!authToken_) { throw new Error("E2E tests require ADMIN_TOKEN/TEST_ADMIN_TOKEN (used to login)"); } ... - Authorization: `Bearer ${authToken}`, - Cookie: `auth-token=${authToken}`, + Authorization: `Bearer ${authToken_}`, + Cookie: `auth-token=${authToken_}`,并将
beforeAll块移至afterAll块之前：+beforeAll(async () => { + if (!ADMIN_KEY) return; + authToken = await loginAndGetAuthToken(API_BASE_URL, ADMIN_KEY); +}); + afterAll(async () => { - if (!sessionToken) return; + if (!authToken) return; ... }); - -beforeAll(async () => { - if (!ADMIN_KEY) return; - sessionToken = await loginAndGetAuthToken(API_BASE_URL, ADMIN_KEY); -});Also applies to: 192-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/users-keys-complete.test.ts` at line 37, Module-level token naming is inconsistent and test hook order is inverted: rename the module-level variable sessionToken to authToken (aligning with other tests) and update all local usages including the callApi fourth parameter named authToken so both refer to the same concept; then move the beforeAll block (currently declared at lines 192–195) to appear before the afterAll block (currently at ~171) so setup runs before teardown and follows the usual hook ordering with beforeAll above afterAll.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (91)
messages/en/dashboard.jsonmessages/en/settings/providers/batchEdit.jsonmessages/en/settings/providers/form/common.jsonmessages/en/settings/providers/form/sections.jsonmessages/ja/dashboard.jsonmessages/ja/settings/providers/batchEdit.jsonmessages/ja/settings/providers/form/common.jsonmessages/ja/settings/providers/form/sections.jsonmessages/ru/dashboard.jsonmessages/ru/settings/providers/batchEdit.jsonmessages/ru/settings/providers/form/common.jsonmessages/ru/settings/providers/form/sections.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/providers/batchEdit.jsonmessages/zh-CN/settings/providers/form/common.jsonmessages/zh-CN/settings/providers/form/sections.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/providers/batchEdit.jsonmessages/zh-TW/settings/providers/form/common.jsonmessages/zh-TW/settings/providers/form/sections.jsonscripts/copy-version-to-standalone.cjssrc/app/[locale]/dashboard/_components/bento/statistics-chart-card.tsxsrc/app/[locale]/dashboard/_components/dashboard-main.tsxsrc/app/[locale]/dashboard/_components/user/add-key-dialog.tsxsrc/app/[locale]/dashboard/_components/user/add-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/batch-edit/batch-edit-dialog.tsxsrc/app/[locale]/dashboard/_components/user/create-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/edit-key-dialog.tsxsrc/app/[locale]/dashboard/_components/user/edit-user-dialog.tsxsrc/app/[locale]/dashboard/_components/user/key-actions.tsxsrc/app/[locale]/dashboard/_components/user/key-list-header.tsxsrc/app/[locale]/dashboard/_components/user/user-actions.tsxsrc/app/[locale]/dashboard/_components/user/user-list.tsxsrc/app/[locale]/dashboard/layout.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsxsrc/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsxsrc/app/[locale]/dashboard/quotas/keys/_components/edit-user-quota-dialog.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-details-tabs.tsxsrc/app/[locale]/dashboard/sessions/_components/session-messages-dialog.tsxsrc/app/[locale]/internal/dashboard/big-screen/loading.tsxsrc/app/[locale]/internal/dashboard/big-screen/page.tsxsrc/app/[locale]/layout.tsxsrc/app/[locale]/login/loading.tsxsrc/app/[locale]/login/page.tsxsrc/app/[locale]/my-usage/layout.tsxsrc/app/[locale]/settings/error-rules/_components/add-rule-dialog.tsxsrc/app/[locale]/settings/error-rules/_components/edit-rule-dialog.tsxsrc/app/[locale]/settings/layout.tsxsrc/app/[locale]/settings/notifications/_components/binding-selector.tsxsrc/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsxsrc/app/[locale]/settings/prices/_components/sync-conflict-dialog.tsxsrc/app/[locale]/settings/providers/_components/adaptive-thinking-editor.tsxsrc/app/[locale]/settings/providers/_components/add-provider-dialog.tsxsrc/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsxsrc/app/[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsxsrc/app/[locale]/settings/providers/_components/batch-edit/provider-batch-preview-step.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/index.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/basic-info-section.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsxsrc/app/[locale]/settings/providers/_components/forms/test-result-card.tsxsrc/app/[locale]/settings/providers/_components/model-multi-select.tsxsrc/app/[locale]/settings/providers/_components/provider-list-item.legacy.tsxsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/[locale]/settings/providers/_components/recluster-vendors-dialog.tsxsrc/app/[locale]/settings/providers/_components/scheduling-rules-dialog.tsxsrc/app/[locale]/settings/providers/_components/thinking-budget-editor.tsxsrc/app/[locale]/settings/providers/_components/vendor-keys-compact-list.tsxsrc/app/[locale]/settings/request-filters/_components/filter-dialog.tsxsrc/app/[locale]/settings/sensitive-words/_components/add-word-dialog.tsxsrc/app/[locale]/settings/sensitive-words/_components/edit-word-dialog.tsxsrc/app/[locale]/usage-doc/layout.tsxsrc/app/global-error.tsxsrc/app/globals.csssrc/components/form/client-restrictions-editor.tsxsrc/components/ui/__tests__/calendar-highlight.test.tsxsrc/components/ui/__tests__/tag-input-dialog.test.tsxsrc/components/ui/drawer.tsxsrc/components/ui/sheet.tsxsrc/components/ui/tag-input.tsxsrc/lib/redis/client.tstests/e2e/_helpers/auth.tstests/e2e/api-complete.test.tstests/e2e/notification-settings.test.tstests/e2e/users-keys-complete.test.tstests/unit/auth/split-set-cookie-header.test.tstests/unit/lib/database-backup/docker-executor.test.tstests/unit/lib/endpoint-circuit-breaker.test.tstests/unit/login/login-visual-regression.test.tsxtests/unit/settings/providers/thinking-budget-editor.test.tsxvitest.config.ts
Comment on lines
+794
to
+799
| "userStatus": { | ||
| "disabled": "已禁用", | ||
| "expired": "已过期", | ||
| "expiringSoon": "即将过期", | ||
| "active": "已启用" | ||
| }, |
There was a problem hiding this comment.
active 翻译值与同文件其他命名空间不一致，请确认是否有意为之
新增的 keyListHeader.userStatus.active 值为 "已启用"，而同文件中 userManagement.userStatus.active（第 1749 行）和 userManagement.keyStatus.active（第 1737 行）的值为 "正常"。
若两处语义有意区分（前者表示"启用状态标签"，后者表示"运行状态"），则当前是正确的。若均表示同一状态，建议统一翻译用词以避免用户界面上出现相互矛盾的提示。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/zh-CN/dashboard.json` around lines 794 - 799,
keyListHeader.userStatus.active translation ("已启用") is inconsistent with
userManagement.userStatus.active and userManagement.keyStatus.active ("正常");
decide whether these represent different semantics (enable/disabled label vs
runtime/healthy status) and then make them consistent: if they refer to the same
state, change keyListHeader.userStatus.active (or the other two) to use the same
wording ("正常" or "已启用"); if they are intentionally different, add a comment in
the JSON or rename the key to clarify intent (e.g.,
keyListHeader.userStatusActiveLabel) and keep translations accordingly. Refer to
keys keyListHeader.userStatus.active, userManagement.userStatus.active, and
userManagement.keyStatus.active to locate and update the entries.
| hideScrollToTop={true} | ||
| hiddenColumns={hideProviderColumn ? ["provider"] : undefined} | ||
| bodyClassName="h-[calc(100vh-56px-32px-40px)]" | ||
| bodyClassName="h-[calc(var(--cch-viewport-height,100vh)-56px-32px-40px)]" |
There was a problem hiding this comment.
修复 calc() 表达式缺少运算符空格导致样式失效
当前 calc(var(--cch-viewport-height,100vh)-56px-32px-40px) 在 CSS 中是无效的，浏览器会忽略该高度，fullscreen 布局可能回退为默认高度。请在 +/- 两侧加空格。
建议修复
- bodyClassName="h-[calc(var(--cch-viewport-height,100vh)-56px-32px-40px)]"
+ bodyClassName="h-[calc(var(--cch-viewport-height,100vh) - 56px - 32px - 40px)]"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| bodyClassName="h-[calc(var(--cch-viewport-height,100vh)-56px-32px-40px)]" | |
| bodyClassName="h-[calc(var(--cch-viewport-height,100vh) - 56px - 32px - 40px)]" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx
at line 479, Fix the invalid CSS calc expression in the bodyClassName string
inside usage-logs-view-virtualized.tsx by adding spaces around the subtraction
operators so the browser parses it; update the bodyClassName value that
currently contains "calc(var(--cch-viewport-height,100vh)-56px-32px-40px)" to
use proper spacing around the "-" operators (e.g.,
"calc(var(--cch-viewport-height, 100vh) - 56px - 32px - 40px)").
Comment on lines
+1
to
+35
| import { describe, expect, it } from "vitest"; | ||
| import { splitSetCookieHeader } from "../../e2e/_helpers/auth"; | ||
|
|
||
| describe("splitSetCookieHeader", () => { | ||
| it("returns empty array for empty input", () => { | ||
| expect(splitSetCookieHeader("")).toEqual([]); | ||
| expect(splitSetCookieHeader(" ")).toEqual([]); | ||
| }); | ||
|
|
||
| it("splits cookies on comma separators", () => { | ||
| expect(splitSetCookieHeader("a=1; Path=/, b=2; Path=/")).toEqual([ | ||
| "a=1; Path=/", | ||
| "b=2; Path=/", | ||
| ]); | ||
| }); | ||
|
|
||
| it("does not split RFC 1123 Expires commas", () => { | ||
| expect( | ||
| splitSetCookieHeader("a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/, b=2; Path=/") | ||
| ).toEqual(["a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/", "b=2; Path=/"]); | ||
| }); | ||
|
|
||
| it("splits when Expires is the last attribute", () => { | ||
| expect(splitSetCookieHeader("a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT, b=2; Path=/")).toEqual( | ||
| ["a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT", "b=2; Path=/"] | ||
| ); | ||
| }); | ||
|
|
||
| it("does not split commas inside quoted cookie values", () => { | ||
| expect(splitSetCookieHeader('a="x, y=z"; Path=/, b=2; Path=/')).toEqual([ | ||
| 'a="x, y=z"; Path=/', | ||
| "b=2; Path=/", | ||
| ]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the auth.ts file referenced in the import
find . -name "auth.ts" -type f | head -20Repository: ding113/claude-code-hub
Length of output: 146
🏁 Script executed:
# Also explore the directory structure
ls -la tests/unit/auth/ 2>/dev/null || echo "Directory not found at tests/unit/auth"
ls -la tests/e2e/_helpers/ 2>/dev/null || echo "Directory not found at tests/e2e/_helpers"Repository: ding113/claude-code-hub
Length of output: 755
🏁 Script executed:
# Search for splitSetCookieHeader implementation
rg "splitSetCookieHeader" --type ts --type tsx -A 30 | head -100Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Search for splitSetCookieHeader implementation
rg "splitSetCookieHeader" -A 30 | head -150Repository: ding113/claude-code-hub
Length of output: 6100
🏁 Script executed:
# Read the auth.ts file at e2e/_helpers location (the source of the function)
cat -n tests/e2e/_helpers/auth.tsRepository: ding113/claude-code-hub
Length of output: 7086
🏁 Script executed:
# Read the test file
cat -n tests/unit/auth/split-set-cookie-header.test.tsRepository: ding113/claude-code-hub
Length of output: 1511
🌐 Web query:
vitest latest stable version 2026
💡 Result:
As of February 24, 2026, the latest stable (npm latest tag) version of Vitest is 4.0.18 (published January 22, 2026). [1][2]
The most recent pre-release is 4.1.0-beta.3 (published February 2, 2026). [3]
Sources: [1] Snyk package page, [2] Socket package overview, [3] Release Alert version feed.
缺少对引号内转义序列的测试覆盖。
splitSetCookieHeader 的 escapeNext 分支（auth.ts 第 23–30 行）负责处理引号内 \" 的转义字符，当前没有任何测试覆盖该逻辑路径。根据编码规范，新特性的单元测试覆盖率需达到 80%。
建议补充测试：
➕ 建议补充测试
+ it("does not split commas in escape-quoted values", () => {
+ expect(splitSetCookieHeader('a="x\\"y, z"; Path=/, b=2; Path=/')).toEqual([
+ 'a="x\\"y, z"; Path=/',
+ "b=2; Path=/",
+ ]);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { describe, expect, it } from "vitest"; | |
| import { splitSetCookieHeader } from "../../e2e/_helpers/auth"; | |
| describe("splitSetCookieHeader", () => { | |
| it("returns empty array for empty input", () => { | |
| expect(splitSetCookieHeader("")).toEqual([]); | |
| expect(splitSetCookieHeader(" ")).toEqual([]); | |
| }); | |
| it("splits cookies on comma separators", () => { | |
| expect(splitSetCookieHeader("a=1; Path=/, b=2; Path=/")).toEqual([ | |
| "a=1; Path=/", | |
| "b=2; Path=/", | |
| ]); | |
| }); | |
| it("does not split RFC 1123 Expires commas", () => { | |
| expect( | |
| splitSetCookieHeader("a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/, b=2; Path=/") | |
| ).toEqual(["a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/", "b=2; Path=/"]); | |
| }); | |
| it("splits when Expires is the last attribute", () => { | |
| expect(splitSetCookieHeader("a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT, b=2; Path=/")).toEqual( | |
| ["a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT", "b=2; Path=/"] | |
| ); | |
| }); | |
| it("does not split commas inside quoted cookie values", () => { | |
| expect(splitSetCookieHeader('a="x, y=z"; Path=/, b=2; Path=/')).toEqual([ | |
| 'a="x, y=z"; Path=/', | |
| "b=2; Path=/", | |
| ]); | |
| }); | |
| }); | |
| import { describe, expect, it } from "vitest"; | |
| import { splitSetCookieHeader } from "../../e2e/_helpers/auth"; | |
| describe("splitSetCookieHeader", () => { | |
| it("returns empty array for empty input", () => { | |
| expect(splitSetCookieHeader("")).toEqual([]); | |
| expect(splitSetCookieHeader(" ")).toEqual([]); | |
| }); | |
| it("splits cookies on comma separators", () => { | |
| expect(splitSetCookieHeader("a=1; Path=/, b=2; Path=/")).toEqual([ | |
| "a=1; Path=/", | |
| "b=2; Path=/", | |
| ]); | |
| }); | |
| it("does not split RFC 1123 Expires commas", () => { | |
| expect( | |
| splitSetCookieHeader("a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/, b=2; Path=/") | |
| ).toEqual(["a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/", "b=2; Path=/"]); | |
| }); | |
| it("splits when Expires is the last attribute", () => { | |
| expect(splitSetCookieHeader("a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT, b=2; Path=/")).toEqual( | |
| ["a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT", "b=2; Path=/"] | |
| ); | |
| }); | |
| it("does not split commas inside quoted cookie values", () => { | |
| expect(splitSetCookieHeader('a="x, y=z"; Path=/, b=2; Path=/')).toEqual([ | |
| 'a="x, y=z"; Path=/', | |
| "b=2; Path=/", | |
| ]); | |
| }); | |
| it("does not split commas in escape-quoted values", () => { | |
| expect(splitSetCookieHeader('a="x\\"y, z"; Path=/, b=2; Path=/')).toEqual([ | |
| 'a="x\\"y, z"; Path=/', | |
| "b=2; Path=/", | |
| ]); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/auth/split-set-cookie-header.test.ts` around lines 1 - 35, The
tests for splitSetCookieHeader are missing coverage for the escapeNext branch
that handles backslash-escaped quotes inside quoted cookie values; add a unit
test in tests/unit/auth/split-set-cookie-header.test.ts that exercises
splitSetCookieHeader with a Set-Cookie string containing an escaped quote
sequence (e.g. a="value with \\\" escaped quote"; Path=/, b=2; Path=/) to ensure
the function's escapeNext logic (from auth.ts handling of escaped characters
inside quotes) correctly preserves the escaped quote and does not split on the
comma; assert the returned array contains the full first cookie including the
escaped quote and the second cookie as before.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR addresses multiple issues including mobile UI viewport适配 (Issue #799), Redis client zombie connections, E2E test stability, and i18n key completeness. The changes are well-structured with proper TypeScript types, error handling, and test coverage.
PR Size: XL
- Lines changed: 1029 additions, 375 deletions
- Files changed: 91
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Observations
-
Mobile UI (dvh) - CSS variables (
--cch-viewport-height-*) properly usedvhfallback via@supportsrule, with graceful degradation tovhfor older browsers. -
Client Restrictions Editor - New component with proper TypeScript interfaces. The
uniqueOrderedhelper function handles deduplication correctly. Mutual exclusion between allowed/blocked lists is properly enforced. -
Redis Client - Type safety improved with explicit return types for
buildRedisOptionsForUrlandbuildTlsConfig. No logic changes that could introduce bugs. -
E2E Tests - The
splitSetCookieHeaderfunction correctly handles RFC 1123 date parsing (commas in Expires attribute), quoted values, and retry logic for transient failures. -
TagInput Fix - Uses capture phase (
trueflag) for mousedown event listener to prevent dropdown from closing when selecting suggestions - this is the correct approach. -
Test Coverage - New unit tests added for cookie parsing. Existing calendar tests fixed for flaky date handling.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate with new tests added
- Code clarity - Good
Automated review by Claude AI
| [getPresetLabel] | ||
| ); | ||
|
|
||
| const handleAllowedChange = (next: string[]) => { |
Contributor
There was a problem hiding this comment.
[HIGH] [TEST-MISSING-CRITICAL] ClientRestrictionsEditor logic has no unit tests
Why this is a problem: uniqueOrdered() plus the cross-list sync in handleAllowedChange/handleBlockedChange (e.g. const nextBlocked = blocked.filter((value) => !allowedSet.has(value));) is new behavior and easy to regress without coverage. Project rule: "All new features must have unit test coverage of at least 80%".
Suggested fix:
// src/components/form/__tests__/client-restrictions-editor.test.tsx
/**
* @vitest-environment happy-dom
*/
import type { ReactNode } from "react";
import React from "react";
import { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, test, vi } from "vitest";
import { ClientRestrictionsEditor } from "@/components/form/client-restrictions-editor";
function render(node: ReactNode) {
const container = document.createElement("div");
document.body.appendChild(container);
const root = createRoot(container);
act(() => {
root.render(node);
});
return {
unmount: () => {
act(() => root.unmount());
container.remove();
},
};
}
afterEach(() => {
while (document.body.firstChild) {
document.body.removeChild(document.body.firstChild);
}
});
describe("ClientRestrictionsEditor", () => {
test("removes from blocked when the same tag is added to allowed", async () => {
const onAllowedChange = vi.fn();
const onBlockedChange = vi.fn();
const { unmount } = render(
<ClientRestrictionsEditor
allowed={[]}
blocked={["claude-code"]}
onAllowedChange={onAllowedChange}
onBlockedChange={onBlockedChange}
allowedLabel="Allowed"
blockedLabel="Blocked"
getPresetLabel={(value) => value}
/>
);
const [allowedInput] = Array.from(document.querySelectorAll("input"));
expect(allowedInput).toBeTruthy();
await act(async () => {
allowedInput?.focus();
await new Promise((resolve) => setTimeout(resolve, 50));
});
const button = Array.from(document.querySelectorAll("button")).find(
(b) => b.textContent === "claude-code"
);
expect(button).toBeTruthy();
await act(async () => {
button?.dispatchEvent(new MouseEvent("mousedown", { bubbles: true }));
await new Promise((resolve) => setTimeout(resolve, 0));
});
expect(onAllowedChange).toHaveBeenCalledWith(["claude-code"]);
expect(onBlockedChange).toHaveBeenCalledWith([]);
unmount();
});
test("removes from allowed when the same tag is added to blocked", async () => {
const onAllowedChange = vi.fn();
const onBlockedChange = vi.fn();
const { unmount } = render(
<ClientRestrictionsEditor
allowed={["claude-code"]}
blocked={[]}
onAllowedChange={onAllowedChange}
onBlockedChange={onBlockedChange}
allowedLabel="Allowed"
blockedLabel="Blocked"
getPresetLabel={(value) => value}
/>
);
const inputs = Array.from(document.querySelectorAll("input"));
const blockedInput = inputs[1];
expect(blockedInput).toBeTruthy();
await act(async () => {
blockedInput?.focus();
await new Promise((resolve) => setTimeout(resolve, 50));
});
const button = Array.from(document.querySelectorAll("button")).find(
(b) => b.textContent === "claude-code"
);
expect(button).toBeTruthy();
await act(async () => {
button?.dispatchEvent(new MouseEvent("mousedown", { bubbles: true }));
await new Promise((resolve) => setTimeout(resolve, 0));
});
expect(onBlockedChange).toHaveBeenCalledWith(["claude-code"]);
expect(onAllowedChange).toHaveBeenCalledWith([]);
unmount();
});
});
| logger.warn("[Redis] Connection closed", { redisUrl: safeRedisUrl }); | ||
| }); | ||
|
|
||
| client.on("end", () => { |
Contributor
There was a problem hiding this comment.
[HIGH] [TEST-MISSING-CRITICAL] Redis singleton reset/close behavior is untested
Why this is a problem: The singleton lifecycle changed (e.g. client.on("end", ... redisClient = null) and closeRedis() now falls back to disconnect() on quit() failure). This is a critical dependency path; without unit tests, regressions here can break rate limiting/session tracking in hard-to-debug ways. Project rule: "All new features must have unit test coverage of at least 80%".
Suggested fix:
// tests/unit/lib/redis/client.test.ts
import { beforeEach, describe, expect, it, vi } from "vitest";
const handlers = new Map<string, Array<(...args: unknown[]) => void>>();
class FakeRedis {
status = "ready";
quit = vi.fn(async () => undefined);
disconnect = vi.fn();
on(event: string, cb: (...args: unknown[]) => void) {
handlers.set(event, [...(handlers.get(event) ?? []), cb]);
return this;
}
emit(event: string, ...args: unknown[]) {
for (const cb of handlers.get(event) ?? []) cb(...args);
}
}
vi.mock("ioredis", () => ({
default: FakeRedis,
}));
vi.mock("@/lib/logger", () => ({
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
}));
async function loadClientModule() {
vi.resetModules();
return await import("@/lib/redis/client");
}
beforeEach(() => {
handlers.clear();
delete process.env.NEXT_PHASE;
process.env.ENABLE_RATE_LIMIT = "true";
process.env.REDIS_URL = "redis://localhost:6379";
});
describe("redis client singleton", () => {
it("resets singleton on end and recreates on next getRedisClient", async () => {
const { getRedisClient } = await loadClientModule();
const client1 = getRedisClient();
expect(client1).toBeTruthy();
(client1 as unknown as FakeRedis).emit("end");
const client2 = getRedisClient();
expect(client2).toBeTruthy();
expect(client2).not.toBe(client1);
});
it("closeRedis disconnects when quit throws and clears singleton", async () => {
const { getRedisClient, closeRedis } = await loadClientModule();
const client = getRedisClient() as unknown as FakeRedis;
client.quit.mockRejectedValueOnce(new Error("quit failed"));
await closeRedis();
expect(client.disconnect).toHaveBeenCalledTimes(1);
expect(getRedisClient()).not.toBe(client);
});
});
Contributor
There was a problem hiding this comment.
Code Review Summary
This is an XL PR (1404 LOC across 91 files) mixing UI layout changes, provider-form behavior changes, Redis lifecycle changes, and test-suite stabilization. The main risk is regression due to new/changed behavior without corresponding unit coverage in a few critical areas.
PR Size: XL
- Lines changed: 1404
- Files changed: 91
- Split suggestions (recommended for follow-up):
- PR 1: viewport-height/safe-area CSS + dialog/layout max-height changes only
- PR 2: provider form client restrictions editor + routing section UX changes + i18n keys
- PR 3: Redis singleton lifecycle fix + E2E auth helper/test stability changes
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 2 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None
High Priority Issues (Should Fix)
src/components/form/client-restrictions-editor.tsx:101[TEST-MISSING-CRITICAL] New allow/block sync logic lacks unit tests (Confidence: 90)src/lib/redis/client.ts:142[TEST-MISSING-CRITICAL] Redis singleton reset/close changes lack unit tests (Confidence: 85)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
Applied size/XL label to PR #822 and submitted the review.
Inline comments added (2):
src/components/form/client-restrictions-editor.tsx:101[HIGH][TEST-MISSING-CRITICAL]Add unit tests for new allow/block sync + normalization logic.src/lib/redis/client.ts:142[HIGH][TEST-MISSING-CRITICAL]Add unit tests for Redis singleton reset-on-endandcloseRedis()quit→disconnect fallback.
- fix(css): add spaces around operators in calc() expression for valid CSS - fix(a11y): replace nested <main> with <div> in root layout to avoid ARIA violation - fix(ui): replace emoji literals with lucide-react icons in scheduling-rules-dialog - fix(i18n): extract hardcoded Chinese strings in big-screen page to i18n keys (5 langs) - fix(test): update outdated comments in calendar-highlight tests - fix(test): add cancelable:true to MouseEvent in tag-input-dialog tests - test: add unit tests for ClientRestrictionsEditor allow/block sync logic - test: add unit tests for Redis singleton reset-on-end and closeRedis fallback
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/__tests__/tag-input-dialog.test.tsx (1)
88-88:⚠️ Potential issue | 🟡 Minor现有测试
MouseEvent缺少cancelable: true，与新测试写法不一致。第 88 行仍使用
{ bubbles: true }而未加cancelable: true，导致组件onMouseDown中的e.preventDefault()为空操作，handleBlur的 150ms 定时器在某些环境下仍可能关闭下拉列表，与新测试（第 120、128 行）的正确写法不一致。🛠️ 建议修复
- suggestionButton?.dispatchEvent(new MouseEvent("mousedown", { bubbles: true })); + suggestionButton?.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/__tests__/tag-input-dialog.test.tsx` at line 88, Update the test so the dispatched MouseEvent includes cancelable: true to match the other tests; specifically change the event created for suggestionButton (the call that does suggestionButton?.dispatchEvent(new MouseEvent("mousedown", { bubbles: true }))) to include { bubbles: true, cancelable: true } so the component's onMouseDown handler can call e.preventDefault() and prevent handleBlur's 150ms timer from closing the dropdown.
🧹 Nitpick comments (1)
tests/unit/components/form/client-restrictions-editor.test.tsx (1)
36-41: 基于调用序号（call index）获取onChange的方式较为脆弱。
getTagInputOnChange(0)/getTagInputOnChange(1)依赖TagInput被渲染的序号。一旦组件内部新增或调整了TagInput的渲染顺序，测试会在没有任何编译错误提示的情况下静默地作用于错误的处理器。可考虑通过 mock 参数（如
value的内容）来区分两个TagInput实例，而非依赖索引：♻️ 建议改进：通过 prop 值定位目标 TagInput
-function getTagInputOnChange(callIndex: number): (values: string[]) => void { - const calls = vi.mocked(TagInput).mock.calls; - const call = calls[callIndex]; - if (!call) throw new Error(`TagInput call ${callIndex} not found (got ${calls.length} calls)`); - return (call[0] as TagInputProps).onChange; -} +function getTagInputOnChangeByValue( + currentValue: string[], +): (values: string[]) => void { + const calls = vi.mocked(TagInput).mock.calls; + const call = calls.find( + (c) => + JSON.stringify((c[0] as TagInputProps).value) === + JSON.stringify(currentValue), + ); + if (!call) throw new Error(`TagInput with value ${JSON.stringify(currentValue)} not found`); + return (call[0] as TagInputProps).onChange; +}测试中改用：
-act(() => getTagInputOnChange(0)(["a", "b", "a", "c"])); +act(() => getTagInputOnChangeByValue([])(["a", "b", "a", "c"]));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/components/form/client-restrictions-editor.test.tsx` around lines 36 - 41, 当前的 getTagInputOnChange 函数基于 TagInput mock 的调用序号（callIndex）来取 onChange，这会因渲染顺序变化而脆弱；请改为通过 TagInput 的 props 来定位目标实例（例如匹配 (call[0] as TagInputProps).value 或其他唯一标识 prop），遍历 vi.mocked(TagInput).mock.calls 查找第一个 props 符合预期值的调用并返回其 onChange，若找不到则抛出带有搜索条件的清晰错误信息；保持函数名 getTagInputOnChange、类型断言到 TagInputProps 及返回类型不变以便兼容现有测试。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/components/form/client-restrictions-editor.test.tsx`:
- Around line 57-134: Tests currently mock CLIENT_RESTRICTION_PRESET_OPTIONS as
[] and TagInput returning null, leaving preset logic and prop passthrough
untested; update tests for ClientRestrictionsEditor to mock
CLIENT_RESTRICTION_PRESET_OPTIONS with non-empty entries and adjust the TagInput
mock to capture/return props so you can assert getPresetLabel is called and that
allowedLabel/blockedLabel are forwarded to TagInput, then add a test that
renders via renderEditor, triggers the preset/suggestion path (e.g., ensure
useMemo produces suggestions) and asserts onPreset label calls and that TagInput
received allowedLabel/blockedLabel to raise coverage above 80%.
- Around line 51-55: 当前 afterEach 只删除 DOM 节点但不卸载 React 根，导致泄漏；在 renderEditor
中将创建的 React 根（如 root）和一个模块级 cleanup 注册出来（例如导出或闭包保存的 cleanup 函数），在 afterEach 调用该
cleanup 或 root.unmount() 来统一卸载，并移除各测试中手动调用的 unmount() 行以避免重复和漏卸载；确保 renderEditor
在创建 root 时设置 cleanup，以便 afterEach 能可靠调用。
---
Outside diff comments:
In `@src/components/ui/__tests__/tag-input-dialog.test.tsx`:
- Line 88: Update the test so the dispatched MouseEvent includes cancelable:
true to match the other tests; specifically change the event created for
suggestionButton (the call that does suggestionButton?.dispatchEvent(new
MouseEvent("mousedown", { bubbles: true }))) to include { bubbles: true,
cancelable: true } so the component's onMouseDown handler can call
e.preventDefault() and prevent handleBlur's 150ms timer from closing the
dropdown.
---
Nitpick comments:
In `@tests/unit/components/form/client-restrictions-editor.test.tsx`:
- Around line 36-41: 当前的 getTagInputOnChange 函数基于 TagInput mock
的调用序号（callIndex）来取 onChange，这会因渲染顺序变化而脆弱；请改为通过 TagInput 的 props 来定位目标实例（例如匹配
(call[0] as TagInputProps).value 或其他唯一标识 prop），遍历 vi.mocked(TagInput).mock.calls
查找第一个 props 符合预期值的调用并返回其 onChange，若找不到则抛出带有搜索条件的清晰错误信息；保持函数名
getTagInputOnChange、类型断言到 TagInputProps 及返回类型不变以便兼容现有测试。
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (13)
messages/en/bigScreen.jsonmessages/ja/bigScreen.jsonmessages/ru/bigScreen.jsonmessages/zh-CN/bigScreen.jsonmessages/zh-TW/bigScreen.jsonsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsxsrc/app/[locale]/internal/dashboard/big-screen/page.tsxsrc/app/[locale]/layout.tsxsrc/app/[locale]/settings/providers/_components/scheduling-rules-dialog.tsxsrc/components/ui/__tests__/calendar-highlight.test.tsxsrc/components/ui/__tests__/tag-input-dialog.test.tsxtests/unit/components/form/client-restrictions-editor.test.tsxtests/unit/lib/redis/client.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx
- src/app/[locale]/layout.tsx
- src/app/[locale]/internal/dashboard/big-screen/page.tsx
Comment on lines
+51
to
+55
| afterEach(() => { | ||
| while (document.body.firstChild) { | ||
| document.body.removeChild(document.body.firstChild); | ||
| } | ||
| }); |
There was a problem hiding this comment.
afterEach 仅清理 DOM，未卸载 React 根节点，存在测试泄漏风险。
当前 afterEach 仅通过 removeChild 移除 DOM 节点，但不调用 root.unmount()。每个测试在末尾手动调用 unmount()，一旦某个断言提前抛出（测试失败），unmount() 就不会执行。被孤立的 React 根节点仍处于活跃状态，可能向后续测试中输出 React 警告，甚至造成状态干扰。
建议将 cleanup 函数集中管理：
🛡️ 建议修复：在 `afterEach` 中统一卸载 React 根节点
+let cleanupFn: (() => void) | null = null;
+
afterEach(() => {
+ cleanupFn?.();
+ cleanupFn = null;
while (document.body.firstChild) {
document.body.removeChild(document.body.firstChild);
}
});同时修改 renderEditor，将 cleanup 注册到模块级变量：
function renderEditor(allowed: string[], blocked: string[]) {
- return render(
+ cleanupFn = render(
<ClientRestrictionsEditor
...
/>
);
}并删除各测试中手动调用 unmount() 的行。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/components/form/client-restrictions-editor.test.tsx` around lines
51 - 55, 当前 afterEach 只删除 DOM 节点但不卸载 React 根，导致泄漏；在 renderEditor 中将创建的 React 根（如
root）和一个模块级 cleanup 注册出来（例如导出或闭包保存的 cleanup 函数），在 afterEach 调用该 cleanup 或
root.unmount() 来统一卸载，并移除各测试中手动调用的 unmount() 行以避免重复和漏卸载；确保 renderEditor 在创建 root
时设置 cleanup，以便 afterEach 能可靠调用。
Comment on lines
+57
to
+134
| function renderEditor(allowed: string[], blocked: string[]) { | ||
| return render( | ||
| <ClientRestrictionsEditor | ||
| allowed={allowed} | ||
| blocked={blocked} | ||
| onAllowedChange={onAllowedChange} | ||
| onBlockedChange={onBlockedChange} | ||
| allowedLabel="Allowed" | ||
| blockedLabel="Blocked" | ||
| getPresetLabel={(v) => v} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| describe("uniqueOrdered normalization", () => { | ||
| it("deduplicates values preserving first occurrence order", () => { | ||
| const unmount = renderEditor([], []); | ||
| act(() => getTagInputOnChange(0)(["a", "b", "a", "c"])); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["a", "b", "c"]); | ||
| unmount(); | ||
| }); | ||
|
|
||
| it("trims whitespace from values", () => { | ||
| const unmount = renderEditor([], []); | ||
| act(() => getTagInputOnChange(0)([" a ", " b", "c "])); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["a", "b", "c"]); | ||
| unmount(); | ||
| }); | ||
|
|
||
| it("filters out empty and whitespace-only entries", () => { | ||
| const unmount = renderEditor([], []); | ||
| act(() => getTagInputOnChange(0)(["a", "", " ", "b"])); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["a", "b"]); | ||
| unmount(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("allow/block mutual exclusion", () => { | ||
| it("removes overlapping items from blocked when added to allowed", () => { | ||
| const unmount = renderEditor([], ["b", "c"]); | ||
| act(() => getTagInputOnChange(0)(["a", "b"])); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["a", "b"]); | ||
| expect(onBlockedChange).toHaveBeenCalledWith(["c"]); | ||
| unmount(); | ||
| }); | ||
|
|
||
| it("does not call onBlockedChange when allowed has no overlap with blocked", () => { | ||
| const unmount = renderEditor([], ["c", "d"]); | ||
| act(() => getTagInputOnChange(0)(["a", "b"])); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["a", "b"]); | ||
| expect(onBlockedChange).not.toHaveBeenCalled(); | ||
| unmount(); | ||
| }); | ||
|
|
||
| it("removes overlapping items from allowed when added to blocked", () => { | ||
| const unmount = renderEditor(["a", "b"], []); | ||
| act(() => getTagInputOnChange(1)(["b", "c"])); | ||
| expect(onBlockedChange).toHaveBeenCalledWith(["b", "c"]); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["a"]); | ||
| unmount(); | ||
| }); | ||
|
|
||
| it("does not call onAllowedChange when blocked has no overlap with allowed", () => { | ||
| const unmount = renderEditor(["a", "b"], []); | ||
| act(() => getTagInputOnChange(1)(["c", "d"])); | ||
| expect(onBlockedChange).toHaveBeenCalledWith(["c", "d"]); | ||
| expect(onAllowedChange).not.toHaveBeenCalled(); | ||
| unmount(); | ||
| }); | ||
|
|
||
| it("clears all blocked when all items are moved to allowed", () => { | ||
| const unmount = renderEditor([], ["x", "y"]); | ||
| act(() => getTagInputOnChange(0)(["x", "y", "z"])); | ||
| expect(onAllowedChange).toHaveBeenCalledWith(["x", "y", "z"]); | ||
| expect(onBlockedChange).toHaveBeenCalledWith([]); | ||
| unmount(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
建议补充对 suggestions/presets 逻辑和 prop 传递的覆盖，以满足 ≥80% 的覆盖率要求。
当前测试套件存在以下几处覆盖缺口，可能导致整体覆盖率低于编码规范要求的 80%：
-
Suggestions 推导路径未覆盖：
CLIENT_RESTRICTION_PRESET_OPTIONS被 mock 为[]，使得组件内部useMemo始终生成空数组，getPresetLabel回调（Line 66）实际上从未被调用。若实现中含有 preset 过滤或映射逻辑（见src/components/form/client-restrictions-editor.tsx），则这些分支完全未被测试。 -
allowedLabel/blockedLabelprop 传递未验证：这两个 prop 仅被传入组件，但由于TagInputmock 返回null，没有任何断言验证标签是否被正确传递给子编辑器。
建议在现有测试基础上补充至少一组使用非空 CLIENT_RESTRICTION_PRESET_OPTIONS 的测试，以覆盖 getPresetLabel 调用路径。
As per coding guidelines: **/*.test.{ts,tsx,js,jsx}: All new features must have unit test coverage of at least 80%.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/components/form/client-restrictions-editor.test.tsx` around lines
57 - 134, Tests currently mock CLIENT_RESTRICTION_PRESET_OPTIONS as [] and
TagInput returning null, leaving preset logic and prop passthrough untested;
update tests for ClientRestrictionsEditor to mock
CLIENT_RESTRICTION_PRESET_OPTIONS with non-empty entries and adjust the TagInput
mock to capture/return props so you can assert getPresetLabel is called and that
allowedLabel/blockedLabel are forwarded to TagInput, then add a test that
renders via renderEditor, triggers the preset/suggestion path (e.g., ensure
useMemo produces suggestions) and asserts onPreset label calls and that TagInput
received allowedLabel/blockedLabel to raise coverage above 80%.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
dvh视口高度与safe-area安全区Test plan
bun run test确认单测全部通过Description enhanced by Claude AI
Greptile Summary
Comprehensive mobile UX improvements and infrastructure fixes across the provider form and testing suite.
Key Changes:
dvhviewport units and safe-area padding. Reordered tab navigation to prevent bottom nav overlap on mobile devices.ClientRestrictionsEditorcomponent with automatic conflict resolution between allowlist/blocklist, integrated TagInput with preset suggestions for common CLI clients (claude-code, gemini-cli, etc).endstatus and recreating connection. Removed CI environment restriction to allow Redis connection during tests. Improved type safety with explicit return types.flex-1 min-w-0) for better mobile responsiveness. Added max-width constraint to model multi-select popover.Confidence Score: 5/5
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Provider Form Mobile View] --> B[CSS: dvh viewport units] A --> C[CSS: safe-area-bottom] A --> D[Layout: Reordered tab nav] E[Routing Section] --> F[ClientRestrictionsEditor] F --> G[TagInput with Suggestions] F --> H[Conflict Resolution Logic] H --> I[Remove from Allowed if in Blocked] H --> J[Remove from Blocked if in Allowed] K[E2E Tests] --> L[loginAndGetAuthToken] L --> M[splitSetCookieHeader] M --> N[Parse Expires RFC 1123] M --> O[Handle Quoted Values] L --> P[Retry Logic] P --> Q[Network Failures] P --> R[503 SESSION_CREATE_FAILED] S[Redis Client] --> T{Check Status} T -->|end| U[Reset Singleton] T -->|ready/connect| V[Return Existing] S --> W[Event Handlers] W --> X[end event: null singleton] W --> Y[close event: log only] Z[TagInput Dropdown] --> AA[Capture Phase Listener] AA --> AB[Prevent False Outside Click] AB --> AC[Keep Open on Suggestion Click]Last reviewed commit: 787adae