Conversation
…#729) * feat: 统一 session_id 格式并注入 Claude metadata.user_id - 修改 generateDeterministicSessionId() 生成格式为 sess_{8位}_{12位} - 为 Claude 请求自动注入 metadata.user_id（格式：user_{hash}_account__session_{sessionId}） - user hash 基于 API Key ID 生成，保持稳定 - 如果已存在 metadata.user_id 则保持原样 * fix: address review feedback and add unit tests for metadata injection * feat(proxy): add configurable claude metadata user_id injection add system setting enableClaudeMetadataUserIdInjection with full config pipeline and UI toggle move injection after private parameter filtering and persist audit special settings for hit/skip reasons add drizzle migration for enable_claude_metadata_user_id_injection --------- Co-authored-by: programcaicai <programcaicai@programcaicaideMac-mini.local>
* feat: rebase PR #701 onto latest dev and resolve migration conflict * fix(lint): sort provider rich item imports * fix(lint): restore file and sort imports * fix: address bot review items in provider priority PR
* feat(security): 用 Vacuum Filter 负向短路保护 API Key 校验 - 新增 VacuumFilter（AR + vacuuming + 插入失败回滚，支持删除）\n- 新增进程级 API Key 过滤器：仅 has==false 才短路，has==true 仍走 DB 校验\n- 启动预热 + 新 key 增量写入；插入失败自动禁用短路并后台重建\n- 添加单测覆盖（正确性/回滚/alt 可逆/短路避免 DB）\n\nEnv: ENABLE_API_KEY_VACUUM_FILTER=true * feat(security): API Key 鉴权加入 Redis 缓存链路 * fix(security): Vacuum Filter 多实例同步与重建窗口修复 * fix(security): 真空过滤器重建期禁用短路 - ApiKeyVacuumFilter：reloading 时返回 null，避免 stale vf 误拒绝\n- VacuumFilter：MurmurHash tail 去掉 switch fallthrough\n- repo/key 与 instrumentation：按 Biome 规则整理 import\n- 测试：补齐 Redis 鉴权缓存 key/TTL/失效覆盖 + reloading 窗口单测\n- 杂项：清理无效 suppression 与多余依赖 * fix(security): 避免多实例新建 API Key 的短暂误拒绝 * fix(security): 加固 API Key 鉴权缓存与 Edge/重建一致性 * feat(security): login 侧复用 VF+Redis 鉴权缓存链路 * fix: 兼容 bun 构建并完善 API Key 缓存开关解析 * fix: 统一 Redis 启用开关解析（兼容 1/0） * fix(security): 消除 pendingKeys 快照竞态窗口 * fix(security): Vacuum Filter 默认关闭并完善健壮性/文档/测试 * fix(security): 登录/会话鉴权补齐用户禁用/过期校验 * refactor(security): 去重 warmup/randomBytes 并完善示例配置与测试 * fix(security): createKey Redis 最佳努力加超时 * feat(security): Vacuum Filter 默认启用 --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com>
* fix(proxy): SSE结束后识别假200错误 * test(proxy): 补充假200检测注释与用例 * feat(session): 支持禁用会话响应体存储 * chore: 更新会话详情存储提示并同步部署模板 * fix(proxy): 补齐假200的 endpoint 失败记录与用例 * fix(proxy): 加固假200检测一致性与脱敏 * fix(proxy): 流式中断按失败结算并改用错误码 * fix(ui): 假200流式失败提示 * fix(ui): ProviderChain 假200提示 * fix(proxy): 非自然结束计入熔断 - streamEndedNormally=false 且非 client abort：记录 provider/endpoint failure\n- 补充注释说明 STORE_SESSION_RESPONSE_BODY 仅影响持久化\n- upstream-error-detection 增补脱敏/截断单测 * chore: 修复 Biome 格式检查 * fix(proxy): 异常流式也消费 deferred meta - 超时/中断/异常路径也调用 finalizeStream，补齐 providerChain 与熔断归因\n- 持久化 providerId 优先使用 deferred meta，避免假200/异常流归因错位\n- upstream-error-detection 增补 options/空白输入用例 * fix(proxy): 收敛假200检测触发条件 --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com>
* fix: sync overrideResponse and overrideStatusCode in syncDefaultErrorRules The update path for isDefault=true rules was missing overrideResponse and overrideStatusCode fields, causing code-level changes to default error override messages to never propagate to the database on restart. * fix(providers): resolve endpoint update and sync issues (#742) - Fix UI consistency: ensure operations return single authoritative result (success/error) - Fix sync logic: prevent aggressive soft-deletion of sibling endpoints - Fix unique constraints: align constraints to ignore soft-deleted rows - Add comprehensive regression tests for direct edit and sync scenarios - Add data repair tool and documentation for troubleshooting * test: add tests for EditEndpointDialog toast behavior * fix(provider-endpoints): apply review consistency follow-ups * fix(ci): satisfy lint checks after dev merge * fix(provider-endpoints): centralize direct edit error codes
…drilldown (#753) Provider ranking adds avgCostPerRequest and avgCostPerMillionTokens with null-safe division guards. Cache-hit ranking gains per-provider expandable model-level breakdown (modelStats) via a second grouped query keyed by billingModelSource. LeaderboardTable now supports generic expandable rows. API route formats new fields immutably via spread pattern. i18n keys added for all 5 locales. 16 new tests cover repository semantics, API formatting, and multi-provider model grouping.
…ations (#755) * refactor(circuit): unify provider-endpoint status semantics Add IncidentSource type and resolveEndpointDisplayStatus() with priority: circuit-open > circuit-half-open > enabled > disabled. 25 tests covering all status combinations. * feat(endpoint): add batch runtime circuit state query Add batchGetEndpointCircuitInfo() action for bulk endpoint circuit state reads, avoiding N+1 queries in table views. * feat(settings): show endpoint runtime circuit and reset action - ProviderEndpointsTable: display circuit state badge + reset button - ProviderManager: endpoint circuits included in filter/counters - ProviderRichListItem: layered badges (key vs endpoint) - Notification: unified provider+endpoint alerts under one toggle - i18n: 5 languages (en, zh-CN, zh-TW, ja, ru) Tests: 28 passed * chore: remove accidentally committed log files * feat(proxy): record endpoint pool exhaustion in provider chain - Add strictBlockCause and endpointFilterStats to chain metadata - New getEndpointFilterStats function for endpoint pool statistics - Record no_endpoint_candidates with full stats when strict policy blocks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #755 review comments from bugbot - EN/RU i18n: fix inconsistent endpoint terminology and missing "Broken" label - provider-endpoints: add .max(500) bound to batch query schema - provider-vendor-view: remove unused endpointCircuitInfo prop - forwarder: remove unsafe decisionContext cast, use typed errorMessage field - endpoint-circuit-breaker: fix providerId semantics (use vendorId) - notifier: add null check for endpointId in dedup key - circuit-breaker template: use strict !== undefined check, add N/A fallback - provider-manager: use Array.isArray guard for endpoint circuit check - provider-chain-formatter: add typeof number guard for stats - tests: fix assertions to match updated API contracts * fix: improve circuit breaker code quality and test assertions - Remove redundant .min(0) from zod array schema - Fix endpoint status source to "provider" when no circuit incident - Stabilize React Query key by sorting endpoint IDs - Log endpoint info lookup failures instead of silently swallowing - Replace .catch(() => {}) with expect().rejects.toThrow() in tests - Improve test descriptions for clarity --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* feat(anthropic): add adaptive thinking override support
Add adaptive thinking as a new option in the Anthropic thinking override
settings on the provider form, supporting the new API format:
- thinking: { type: "adaptive" } + output_config: { effort: "low"|"medium"|"high"|"max" }
- Model-level targeting: apply to all models or specific models with prefix matching
- Full audit trail tracking output_config.effort changes
- Rectifier safety: skip budget rectification for adaptive mode
- DB: new JSONB column anthropic_adaptive_thinking on providers table
- i18n: all 5 locales (en, zh-CN, zh-TW, ja, ru)
- Tests: 10 override tests + 5 rectifier tests (56/56 passing)
* fix: address bugbot review comments on adaptive thinking PR
1. Add anthropic_adaptive_thinking, anthropic_max_tokens_preference, and
anthropic_thinking_budget_preference to addProvider/editProvider type
signatures for IDE autocomplete and compile-time safety.
2. Add .refine() validation ensuring models array is non-empty when
modelMatchMode is "specific" in adaptive thinking config schema.
* fix(migration): use SQL NULL instead of JSONB null for default value
Change DEFAULT 'null'::jsonb to DEFAULT NULL in the migration for
anthropic_adaptive_thinking column. JSONB null and SQL NULL have
different semantics in queries and indexing.
* fix(proxy): prevent hang on non-ok response body * fix(provider-endpoints): avoid cleanup timestamptz parse errors * chore(proxy-forwarder): fix garbled comment * fix(proxy): restore response timeout after proxy direct fallback * chore: address review comments * chore: 处理 AI 审核建议（测试清理/CAST） --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com>
… breaker and fallback When upstream returns non-200 HTTP status codes (e.g., 400, 500), the proxy now correctly: - Parses JSON error responses to extract error messages - Records failures in circuit breaker for both provider and endpoint - Updates provider chain with error details for logging - Enables proper fallback mechanism to trigger on provider errors Previously, non-200 responses were not triggering circuit breaker recording, causing the decision chain to hang in "requesting" state and disabling the fallback mechanism. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rcuit reset UX Replace HTTP HEAD/GET endpoint probes with pure TCP connection probes (net.createConnection) to avoid leaving access logs on upstream servers. ENDPOINT_PROBE_METHOD env var controls the method (TCP/HEAD/GET, default TCP). Move circuit reset button from dropdown menu to action bar for faster access when endpoints are tripped. Add optimistic update to immediately reflect closed state on reset.
…preference Previously, `anthropicThinkingBudgetPreference` carried triple duty: "inherit" / numeric string / "adaptive", making adaptive thinking and thinking budget mutually exclusive. This change makes them independent configurations that can coexist. Priority order: adaptive thinking (model match) > thinking budget override > inherit client request. Changes: - Remove "adaptive" literal from AnthropicThinkingBudgetPreference type - Decouple validation: adaptive config validated independently - Core logic: dual-path with fallback (adaptive -> budget -> inherit) - UI: separate Switch toggle for adaptive thinking - i18n: update all 5 language files - Migration: clean up legacy 'adaptive' values in budget preference - Tests: update existing + add 6 new coexistence tests (57 total)
The field was omitted from findProviderList, findAllProvidersFresh, and findProviderById, causing the adaptive thinking setting to not persist after page refresh and never take effect in the proxy runtime.
* fix(proxy): 修复 Gemini 流式透传超时链路卡死 - Gemini SSE 透传不再在仅收到 headers 时清除首字节超时 - 首块数据到达后清除首字节超时，并支持 streamingIdleTimeoutMs 静默超时中断 - stats 任务失败时尽量落库/结束追踪，避免请求长期停留在 requesting - 添加回归测试覆盖无首块/首块后延迟/中途静默三种场景 * chore: format code (fix-hang-stuck-requesting-v2-9058885) * fix(proxy): 修复请求卡死（AgentPool 驱逐阻塞 + 流式透传健壮性） - AgentPool 驱逐时优先 destroy，避免 close 等待 in-flight 导致 getAgent/cleanup 卡住\n- Gemini SSE 透传 stats 读取增加内存上限、完善 abort reason 判定与清理日志\n- 补齐/加强回归测试 * test(proxy): 补齐 AgentPool 驱逐卡死回归用例 * fix(proxy): 按 AI review 加固 stats 读取/清理 * test(proxy): 回归用例结束后恢复 fake timers * fix(proxy): AgentPool 驱逐不等待 destroy/close - closeAgent 触发 destroy/close 后不 await，避免驱逐路径被卡住\n- Gemini 透传 stats：仅在收到非空 chunk 后清首字节超时\n- 回归测试：覆盖 close 无 destroy 且永不返回的场景 * test(proxy): 强化 idle watchdog 与 mock 隔离 - 透传 stats：done=false 时也会重置 idle timer（避免 value 异常导致 watchdog 不工作）\n- 回归测试：reset hoisted isHttp2Enabled mock，避免跨用例污染 * test(proxy): 修复 AgentPool shutdown 用例 close mock * test: 稳定 endpoint-circuit-breaker 告警用例（避免 CI/bun 串扰） * test: 彻底避免 endpoint-circuit-breaker 异步告警串扰（CI/bun） --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* perf(vacuum-filter): 优化 has 热路径（编码/哈希/取模） * chore: format code (perf-vacuum-filter-has-61c0d01) * fix(vacuum-filter): 修复 scratch32 对齐崩溃并补齐测试 * chore: format code (perf-vacuum-filter-has-201cf12) * test(bench): 添加 VacuumFilter.has microbench（默认跳过） * chore: format code (perf-vacuum-filter-has-81b8ca6) * perf(vacuum-filter): 明确 fast-reduce 截断并补齐索引不越界测试 * chore: format code (perf-vacuum-filter-has-cd12943) * fix(vacuum-filter): 通过 CI lint 并澄清 fast-reduce 截断语义 * test(circuit): 修复 endpoint 告警用例串台导致的 flake --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Three bugs caused a permanent open circuit death spiral: 1. recordEndpointFailure reset circuitOpenUntil on every call while open 2. Probe success never fed back into the circuit breaker 3. failureCount never reset because open state was never exited Now: timer only sets on initial transition, probe success resets the circuit via getEndpointCircuitStateSync + resetEndpointCircuit, and added tests for death spiral prevention and sync state accessor.
When a session switches models mid-conversation, the previously bound provider may not support the new model. Due to SET NX semantics, the stale binding would persist indefinitely, causing repeated provider rejection without recovery. Clear the binding on model mismatch so a new SET NX can rebind to a compatible provider.
Cover the findReusable() behavior when session-bound provider doesn't support the requested model, verifying clearSessionProvider is called to break the SET NX deadlock cycle.
|
Important Review skippedToo many files! This PR contains 176 files, which is 26 over the limit of 150. You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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! This release, v0.5.5, delivers significant enhancements to API key management, provider endpoint stability, and streaming response processing. It introduces new configuration options for Anthropic models, refines deployment practices, and improves the overall user experience through better error handling and UI responsiveness. The changes aim to bolster security, optimize performance, and provide more accurate operational insights. Highlights
Changelog
Ignored Files
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
|
github-actions
bot
added
enhancement
![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
This is an impressive release pull request that includes a wide range of features, bug fixes, and significant stability improvements. The changes are well-structured and of high quality. Key improvements include the optimization of the Docker build process, enhanced robustness in the proxy layer with features like deferred stream finalization to handle 'fake 200' errors and non-blocking agent pool cleanup, and more granular provider controls with group-based priorities and adaptive thinking settings. The UI has also been made more responsive. I've identified a minor maintainability issue regarding the use of mixed languages in code comments, which should be addressed for consistency. Overall, this is an excellent set of changes.
Comment on lines
+251
to
+260
| * 为 Claude 请求注入 metadata.user_id | ||
| * | ||
| * 格式：user_{stableHash}_account__session_{sessionId} | ||
| * - stableHash: 基于 API Key ID 生成的稳定哈希（64位 hex），生成后保持不变 | ||
| * - sessionId: 当前请求的 session ID | ||
| * | ||
| * 注意：如果请求体中已存在 metadata.user_id，则保持原样不修改 | ||
| * @internal | ||
| */ | ||
| export function injectClaudeMetadataUserId( |
Contributor
There was a problem hiding this comment.
The JSDoc for this function is in Chinese. To maintain a consistent and accessible codebase for all contributors, please write all code comments and documentation in English.
| @@ -162,6 +162,7 @@ export const providers = pgTable('providers', { | |||
|
|
|||
| // 优先级和分组配置 | |||
| priority: integer('priority').notNull().default(0), | |||
Contributor
There was a problem hiding this comment.
This file, along with other parts of the codebase, contains comments in Chinese. For instance, here and on lines 630-631. To ensure consistency and make the code accessible to a broader audience, it's recommended to use English for all code comments.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 11-15: Remove the unnecessary quotes from
ENABLE_API_KEY_VACUUM_FILTER so it matches dotenv-linter's no-quote rule for
boolean envs, and move ENDPOINT_PROBE_METHOD (wrap it in quotes) so it appears
before ENDPOINT_PROBE_TIMEOUT_MS to match the expected ordering; apply the same
quoting/ordering fixes to the other affected env entries referenced (the blocks
around lines 66-69, 77-80, and 153-154) so string-valued variables are quoted
and boolean/numeric flags are unquoted and keys follow the same ordering
convention.
In `@drizzle/0066_hot_mauler.sql`:
- Line 1: The migration adds a JSONB column but uses DEFAULT NULL which breaks
the established JSONB-default pattern; update the ALTER TABLE statement for the
"providers" table to add the "anthropic_adaptive_thinking" column with the JSONB
default expression jsonb DEFAULT 'null'::jsonb (i.e., replace DEFAULT NULL with
jsonb DEFAULT 'null'::jsonb) so the migration matches existing drizzle migration
conventions.
In `@messages/en/settings/providers/list.json`:
- Around line 4-5: The displayed labels for keyCircuitBroken and
endpointCircuitBroken are missing the word "Broken" and should match the other
copy; update the values for the unique keys "keyCircuitBroken" and
"endpointCircuitBroken" in messages/en/settings/providers/list.json to include "
Broken" (e.g., "Key Circuit Broken" and "Endpoint Circuit Broken") so the
terminology is consistent across the page.
In `@messages/ru/provider-chain.json`:
- Line 53: The value for the "endpoint_pool_exhausted" message contains a
Russian grammar error; update the JSON entry for the key
"endpoint_pool_exhausted" to the corrected string "Пул конечных точек исчерпан"
(replace "Пул конечная точкаов исчерпан" with the corrected phrase).
- Around line 67-68: Fix the Russian translation for the JSON key
"endpoint_circuit_open" by replacing the incorrect string "Автомат конечная
точкаа открыт" with the proper phrase "Автомат конечной точки открыт"; leave
"endpoint_disabled" unchanged ("Эндпоинт отключен") and ensure the corrected
value is valid JSON string content for the key "endpoint_circuit_open".
- Line 41: The translation for the JSON key "endpointPoolExhausted" contains a
Russian grammar error; update its value from "Пул конечная точкаов исчерпан" to
the correct form "Пул конечных точек исчерпан" so the plural genitive of
"конечная точка" is used properly.
- Around line 193-199: The Russian translations for the message keys
endpointPoolExhausted, endpointStats, endpointStatsTotal, endpointStatsEnabled,
endpointStatsCircuitOpen, endpointStatsAvailable, and strictBlockNoEndpoints use
incorrect inflections of "конечная точка"; update these strings to use correct
forms (replace "конечная точкаов" with "конечных точек" and "конечная точкаы"
with "конечные точки") or alternatively standardize on the borrowed form
"эндпоинт/эндпоинтов" consistently across these keys to match line 68; have a
native Russian reviewer confirm the final phrasing for grammatical correctness
and consistency.
In `@scripts/deploy.sh`:
- Around line 578-582: Change the default for STORE_SESSION_RESPONSE_BODY from
true to false to minimize retained sensitive data; update the
SESSION_TTL/STORE_SESSION_MESSAGES block so STORE_SESSION_RESPONSE_BODY=false
and add a short comment that response body persistence is opt‑in only, and
ensure any deployment/docs or environment sample variables reference this new
default and explain how to opt in if needed.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx:
- Around line 195-199: The default hardcoded model for the
anthropicAdaptiveThinking state is out of date; update the models array in the
anthropicAdaptiveThinking default from "claude-opus-4-6" to the currently
supported "claude-opus-4-5" (leave effort, modelMatchMode, and the surrounding
structure unchanged) so the default aligns with the system-supported model
version.
In `@src/app/`[locale]/settings/providers/_components/provider-list.tsx:
- Line 7: The import of EndpointCircuitInfoMap currently uses a relative path
("./provider-manager"); replace that relative import with the project path-alias
form starting with "@/..." so the module import for EndpointCircuitInfoMap uses
the `@/` alias instead of "./provider-manager" (update the import statement that
references EndpointCircuitInfoMap accordingly).
In `@src/app/`[locale]/settings/providers/_components/provider-manager.tsx:
- Around line 103-116: The hasAnyCircuitOpen callback currently only treats
circuitState === "open" as a problem; update its checks in hasAnyCircuitOpen to
also treat "half-open" as an exceptional state by checking for circuitState ===
"open" || circuitState === "half-open" (both for the provider-level
healthStatus[providerId]?.circuitState check and the endpoint-level
endpoints.some(...) check) so that both provider and endpoint circuit
evaluations include half-open in their detection and counting.
In `@src/app/`[locale]/settings/providers/_components/provider-rich-list-item.tsx:
- Around line 120-122: The props handling allows a mismatch between isAdmin and
canEdit which can show edit UI while GroupEditCombobox enforces non-admin
restrictions; update the provider-rich-list-item component to derive canEdit
from isAdmin (or fallback so both are consistent) — e.g., ensure the local
canEdit value used to render edit controls is computed from the incoming isAdmin
prop (or default to false when isAdmin is undefined) and pass that same computed
flag into GroupEditCombobox and any other edit-related logic (references:
isAdmin, canEdit, GroupEditCombobox, provider-rich-list-item render logic) so
the UI and selection restrictions remain in sync.
- Around line 585-589: The mobile edit icon button lacks an accessibility label;
update the Button JSX in provider-rich-list-item.tsx (the Button that calls
handleEdit) to include an aria-label prop using the same i18n key pattern used
by the actionClone/actionDelete buttons (reuse their translation key for the
"edit" action), so screen readers can announce the control; ensure the
aria-label value comes from the existing i18n lookup rather than a hardcoded
string.
- Around line 66-68: The new relative imports for GroupEditCombobox,
InlineEditPopover, and PriorityEditPopover do not follow the repo's '@/...'
path-alias convention; update the import statements in
provider-rich-list-item.tsx to use the alias (e.g.,
'@/app/[locale]/settings/providers/_components/GroupEditCombobox', etc.) so all
three imports (GroupEditCombobox, InlineEditPopover, PriorityEditPopover)
reference files via the '@/...' path alias.
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 2506-2520: Remove the emoji characters from the comment block
above the try/finally so the file conforms to the TS no-emoji rule; keep the
comment text and intent intact (explain not to clear responseTimeoutId before
reading response body and to clear it in the finally block), and ensure related
identifiers (ProxyError.fromUpstreamResponse, response, responseTimeoutId, and
the finally cleanup) remain unchanged so behavior is unaffected.
- Around line 214-415: The current injectClaudeMetadataUserId function uses
crypto.createHash("sha256") on keyId which allows keyId enumeration; change the
stableHash generation to use an HMAC with a server-side secret (pepper) — e.g.,
crypto.createHmac("sha256",
serverSecret).update(`claude_user_${keyId}`).digest("hex") — where serverSecret
is read from a secure config/env and remains constant across restarts; update
injectClaudeMetadataUserId (and any callers like
applyClaudeMetadataUserIdInjectionWithAudit) to use that HMAC-based stableHash,
ensure the secret is injected/accessible (e.g., via existing config/provider),
and keep the rest of the user_id format unchanged so downstream behavior stays
stable.
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Line 27: The import of consumeDeferredStreamingFinalization currently uses a
relative path ("./stream-finalization"); replace it with the repository
path-alias import '@/app/v1/_lib/proxy/stream-finalization' to follow the
project convention. Locate the import statement that brings in
consumeDeferredStreamingFinalization and update the module specifier to the
alias form so all local module imports consistently use '@/...' instead of
relative paths.
In `@src/lib/provider-endpoints/endpoint-selector.ts`:
- Around line 56-86: The JSDoc for getEndpointFilterStats incorrectly states it
"Returns null" on raw query failure while the implementation and signature
always return EndpointFilterStats; either update the comment to say the function
will throw if findProviderEndpointsByVendorAndType fails (and mention it queries
findProviderEndpointsByVendorAndType and uses isEndpointCircuitOpen), or change
the signature to Promise<EndpointFilterStats | null> and wrap the call to
findProviderEndpointsByVendorAndType in a try/catch that returns null on error;
make the corresponding adjustment to the EndpointFilterStats usage and tests
accordingly.
🧹 Nitpick comments (12)
src/app/v1/_lib/proxy/thinking-budget-rectifier.ts (1)
64-83: 建议提取currentThinkingBudgetTokens到提前返回之前以消除重复代码。
budget_tokens的提取逻辑在第 71-72、77-78 行与第 82-83 行重复。将currentThinkingBudgetTokens的提取移到adaptive检查之前可以简化代码并提高可维护性。♻️ 建议的重构
const currentThinkingType = thinking && typeof thinking.type === "string" ? thinking.type : null; + const currentThinkingBudgetTokens = + thinking && typeof thinking.budget_tokens === "number" ? thinking.budget_tokens : null; if (currentThinkingType === "adaptive") { return { applied: false, before: { maxTokens: currentMaxTokens, thinkingType: currentThinkingType, - thinkingBudgetTokens: - thinking && typeof thinking.budget_tokens === "number" ? thinking.budget_tokens : null, + thinkingBudgetTokens: currentThinkingBudgetTokens, }, after: { maxTokens: currentMaxTokens, thinkingType: currentThinkingType, - thinkingBudgetTokens: - thinking && typeof thinking.budget_tokens === "number" ? thinking.budget_tokens : null, + thinkingBudgetTokens: currentThinkingBudgetTokens, }, }; } - const currentThinkingBudgetTokens = - thinking && typeof thinking.budget_tokens === "number" ? thinking.budget_tokens : null; const before = {src/lib/auth.ts (1)
117-123: 用户状态校验与validateApiKeyAndGetUser内部逻辑重复。根据
src/repository/key.ts中validateApiKeyAndGetUser的实现，该函数内部已经对user.isEnabled和user.expiresAt进行了校验，若用户禁用或过期会直接返回null。因此这里的检查永远不会触发。如果这是防御性编程的有意设计，建议添加注释说明；否则可以考虑移除以减少冗余代码。
♻️ 如为防御性编程，建议添加注释说明
- // 用户状态校验：与 v1 proxy 侧保持一致，避免禁用/过期用户继续登录或持有会话 + // 防御性校验：validateApiKeyAndGetUser 内部已做同样检查，此处保留以与 v1 proxy 侧保持一致 if (!user.isEnabled) { return null; }src/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsx (1)
544-551: 建议：提取缓存命中率颜色阈值逻辑颜色阈值逻辑在主表格（第 316-321 行）和展开内容（第 546-551 行）中重复出现。可考虑提取为共享函数以便于维护。
♻️ 可选重构：提取颜色阈值函数
// 可在文件顶部或工具模块中定义 const getCacheHitRateColorClass = (rate: number): string => { if (rate >= 85) return "text-green-600 dark:text-green-400"; if (rate >= 60) return "text-yellow-600 dark:text-yellow-400"; return "text-orange-600 dark:text-orange-400"; };src/app/[locale]/dashboard/leaderboard/_components/leaderboard-table.tsx (1)
238-243: 可选改进：增强可访问性可展开行当前仅支持鼠标点击。为提升可访问性，可考虑添加键盘支持和 ARIA 属性。
♿ 可选：增强可访问性
<TableRow className={`${isTopThree ? "bg-muted/50" : ""} ${hasExpandable && expandedContent ? "cursor-pointer" : ""}`} + tabIndex={hasExpandable && expandedContent ? 0 : undefined} + aria-expanded={hasExpandable && expandedContent ? isExpanded : undefined} onClick={ hasExpandable && expandedContent ? () => toggleRow(rowKey) : undefined } + onKeyDown={ + hasExpandable && expandedContent + ? (e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + toggleRow(rowKey); + } + } + : undefined + } >src/lib/proxy-agent/agent-pool.ts (1)
358-398: 非阻塞清理逻辑设计合理。防御性空值检查和优先使用
destroy()而非close()的策略是正确的——undici的close()会等待进行中的请求完成，可能导致阻塞。有一点可以考虑优化：由于该方法不再
await任何操作，可以移除async关键字使方法签名更清晰地反映其非阻塞行为。可选：移除不必要的 async 关键字
- private async closeAgent(agent: Dispatcher, key: string): Promise<void> { + private closeAgent(agent: Dispatcher, key: string): void {相应地，调用处的
await也可以移除，但保留await也不会造成问题。src/lib/provider-endpoints/probe.ts (1)
109-151: 建议：考虑添加 DNS 解析错误的区分处理。当前实现将所有 socket 错误统一归类为
network_error。对于ENOTFOUND（DNS 解析失败）等特定错误，可以提供更精确的errorType以便于问题诊断和监控。可选的改进方案
socket.on("error", (error) => { const latencyMs = Date.now() - start; + const errorCode = (error as NodeJS.ErrnoException).code; + const errorType = errorCode === "ENOTFOUND" ? "dns_error" : "network_error"; logger.debug("[EndpointProbe] TCP probe failed", { url: safeUrlForLog(rawUrl), errorMessage: error.message, }); socket.destroy(); resolve({ ok: false, method: "TCP", statusCode: null, latencyMs, - errorType: "network_error", + errorType, errorMessage: error.message, }); });src/app/v1/_lib/proxy/provider-selector.ts (1)
918-934: 考虑缓存 effectivePriority 计算结果以减少重复计算。在同一选择流程中，
resolveEffectivePriority被多次调用（lines 925-926, 931-933）。虽然当前性能影响不大，但如果 provider 数量增加，可以考虑使用 Map 缓存计算结果。// 可选优化示例 const priorityCache = new Map<number, number>(); const getEffectivePriority = (p: Provider) => { if (!priorityCache.has(p.id)) { priorityCache.set(p.id, ProxyProviderResolver.resolveEffectivePriority(p, effectiveGroupPick ?? null)); } return priorityCache.get(p.id)!; };src/app/[locale]/settings/providers/_components/group-edit-combobox.tsx (1)
118-140: 优化更新模式实现正确，但考虑防止快速点击导致的竞态条件。optimistic update 模式实现良好，失败时正确回滚。但当用户快速连续点击时，多个并发的
onSave调用可能导致状态不一致。建议在
saving状态为true时禁用点击，或使用防抖/节流：📝 可选改进
const toggleGroup = async (group: string) => { + if (saving) return; // 防止快速连续点击 const previousSelection = [...selectedGroups];src/components/ui/drawer.tsx (1)
45-64:DrawerPortal上的data-slot属性重复。
DrawerPortal函数（第 16-18 行）已经添加了data-slot="drawer-portal"，但在DrawerContent中调用时（第 46 行）又传递了相同的属性，这会导致属性被覆盖但没有实际效果。建议移除重复的 data-slot
- <DrawerPortal data-slot="drawer-portal"> + <DrawerPortal>src/actions/provider-endpoints.ts (1)
587-597: 批量查询建议容错单点失败Promise.all 任一 endpoint 抛错会导致整批失败；如需最大化返回，可考虑 Promise.allSettled 或单点 try/catch 返回部分结果。
src/app/[locale]/settings/providers/_components/endpoint-status.ts (1)
22-166: 显示状态建议收窄类型并明确探测语义EndpointDisplayStatus.status 目前为 string，且 resolveEndpointDisplayStatus 未使用 lastProbeOk；如需反映探测失败，建议补充分支并将 status 收窄为字面量联合类型以提升类型安全。
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx (1)
133-149: 客户端使用 toSorted 需确认运行环境支持
toSorted 需要较新的运行时；若需兼容旧浏览器，建议改为复制后排序以避免运行时异常。替换为更广泛兼容的排序方式
- queryKey: ["endpoint-circuit-info", endpointIds.toSorted((a, b) => a - b).join(",")], + queryKey: [ + "endpoint-circuit-info", + [...endpointIds].sort((a, b) => a - b).join(","), + ],
Comment on lines
+11
to
+15
| # API Key Vacuum Filter（真空过滤器） | ||
| # - true (默认)：启用。用于在访问 DB 前“负向短路”无效 key，降低 DB 压力、抵御爆破 | ||
| # - false：禁用（例如：需要排查问题或节省内存时） | ||
| ENABLE_API_KEY_VACUUM_FILTER="true" | ||
|
|
There was a problem hiding this comment.
对新增环境变量的引号/顺序做一次 dotenv-linter 对齐
静态分析提示新增项存在引号与顺序规则不一致的问题。建议移除 ENABLE_API_KEY_VACUUM_FILTER 的引号，并将 ENDPOINT_PROBE_METHOD 作为字符串加引号且放在 ENDPOINT_PROBE_TIMEOUT_MS 之前，以避免 CI 报警。
建议修改
-ENABLE_API_KEY_VACUUM_FILTER="true"
+ENABLE_API_KEY_VACUUM_FILTER=true
@@
-ENDPOINT_PROBE_INTERVAL_MS=60000
-ENDPOINT_PROBE_TIMEOUT_MS=5000
-ENDPOINT_PROBE_CONCURRENCY=10
-ENDPOINT_PROBE_CYCLE_JITTER_MS=1000
-ENDPOINT_PROBE_LOCK_TTL_MS=30000
-# Probe method: TCP (default, no HTTP request / no access log), HEAD, GET
-ENDPOINT_PROBE_METHOD=TCP
+ENDPOINT_PROBE_INTERVAL_MS=60000
+# Probe method: TCP (default, no HTTP request / no access log), HEAD, GET
+ENDPOINT_PROBE_METHOD="TCP"
+ENDPOINT_PROBE_TIMEOUT_MS=5000
+ENDPOINT_PROBE_CONCURRENCY=10
+ENDPOINT_PROBE_CYCLE_JITTER_MS=1000
+ENDPOINT_PROBE_LOCK_TTL_MS=30000Also applies to: 66-69, 77-80, 153-154
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
In @.env.example around lines 11 - 15, Remove the unnecessary quotes from
ENABLE_API_KEY_VACUUM_FILTER so it matches dotenv-linter's no-quote rule for
boolean envs, and move ENDPOINT_PROBE_METHOD (wrap it in quotes) so it appears
before ENDPOINT_PROBE_TIMEOUT_MS to match the expected ordering; apply the same
quoting/ordering fixes to the other affected env entries referenced (the blocks
around lines 66-69, 77-80, and 153-154) so string-valued variables are quoted
and boolean/numeric flags are unquoted and keys follow the same ordering
convention.
| @@ -0,0 +1,8 @@ | |||
| ALTER TABLE "providers" ADD COLUMN "anthropic_adaptive_thinking" jsonb DEFAULT NULL; | |||
There was a problem hiding this comment.
JSONB 默认值需保持 'null'::jsonb 既有模式
当前使用 DEFAULT NULL 与既有迁移模式不一致，建议改回 jsonb DEFAULT 'null'::jsonb 以保持一致性。
✅ 建议修改
-ALTER TABLE "providers" ADD COLUMN "anthropic_adaptive_thinking" jsonb DEFAULT NULL;
+ALTER TABLE "providers" ADD COLUMN "anthropic_adaptive_thinking" jsonb DEFAULT 'null'::jsonb;Based on learnings: Maintain the existing JSONB default pattern jsonb DEFAULT 'null'::jsonb in drizzle migrations (e.g., files under drizzle/). Do not switch to DEFAULT NULL even if the Drizzle schema uses .default(null); the code paths do not rely on NULL vs JSON null distinction and keeping the established pattern ensures consistency across migrations.
📝 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
| ALTER TABLE "providers" ADD COLUMN "anthropic_adaptive_thinking" jsonb DEFAULT NULL; | |
| ALTER TABLE "providers" ADD COLUMN "anthropic_adaptive_thinking" jsonb DEFAULT 'null'::jsonb; |
🤖 Prompt for AI Agents
In `@drizzle/0066_hot_mauler.sql` at line 1, The migration adds a JSONB column but
uses DEFAULT NULL which breaks the established JSONB-default pattern; update the
ALTER TABLE statement for the "providers" table to add the
"anthropic_adaptive_thinking" column with the JSONB default expression jsonb
DEFAULT 'null'::jsonb (i.e., replace DEFAULT NULL with jsonb DEFAULT
'null'::jsonb) so the migration matches existing drizzle migration conventions.
Comment on lines
+4
to
+5
| "keyCircuitBroken": "Key Circuit", | ||
| "endpointCircuitBroken": "Endpoint Circuit", |
There was a problem hiding this comment.
建议与“Circuit Broken”保持用语一致。 目前 “Key Circuit / Endpoint Circuit” 少了 “Broken”，可能与同页面其他文案不一致，影响理解。
💡 建议改动
- "keyCircuitBroken": "Key Circuit",
- "endpointCircuitBroken": "Endpoint Circuit",
+ "keyCircuitBroken": "Key Circuit Broken",
+ "endpointCircuitBroken": "Endpoint Circuit Broken",📝 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
| "keyCircuitBroken": "Key Circuit", | |
| "endpointCircuitBroken": "Endpoint Circuit", | |
| "keyCircuitBroken": "Key Circuit Broken", | |
| "endpointCircuitBroken": "Endpoint Circuit Broken", |
🤖 Prompt for AI Agents
In `@messages/en/settings/providers/list.json` around lines 4 - 5, The displayed
labels for keyCircuitBroken and endpointCircuitBroken are missing the word
"Broken" and should match the other copy; update the values for the unique keys
"keyCircuitBroken" and "endpointCircuitBroken" in
messages/en/settings/providers/list.json to include " Broken" (e.g., "Key
Circuit Broken" and "Endpoint Circuit Broken") so the terminology is consistent
across the page.
| "http2Fallback": "Откат HTTP/2", | ||
| "clientError": "Ошибка клиента" | ||
| "clientError": "Ошибка клиента", | ||
| "endpointPoolExhausted": "Пул конечная точкаов исчерпан" |
There was a problem hiding this comment.
俄语翻译存在语法错误。
"Пул конечная точкаов исчерпан" 中的 "конечная точкаов" 语法不正确。俄语中 "конечная точка"（端点）是阴性名词，正确的复数属格形式应为 "конечных точек"。
建议修正为："Пул конечных точек исчерпан"
📝 建议修复
- "endpointPoolExhausted": "Пул конечная точкаов исчерпан"
+ "endpointPoolExhausted": "Пул конечных точек исчерпан"📝 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
| "endpointPoolExhausted": "Пул конечная точкаов исчерпан" | |
| "endpointPoolExhausted": "Пул конечных точек исчерпан" |
🤖 Prompt for AI Agents
In `@messages/ru/provider-chain.json` at line 41, The translation for the JSON key
"endpointPoolExhausted" contains a Russian grammar error; update its value from
"Пул конечная точкаов исчерпан" to the correct form "Пул конечных точек
исчерпан" so the plural genitive of "конечная точка" is used properly.
| "session_reuse": "Повторное использование сессии", | ||
| "initial_selection": "Первоначальный выбор" | ||
| "initial_selection": "Первоначальный выбор", | ||
| "endpoint_pool_exhausted": "Пул конечная точкаов исчерпан" |
There was a problem hiding this comment.
同上，俄语语法错误。
与 line 41 相同的问题，"Пул конечная точкаов исчерпан" 应修正为 "Пул конечных точек исчерпан"。
🤖 Prompt for AI Agents
In `@messages/ru/provider-chain.json` at line 53, The value for the
"endpoint_pool_exhausted" message contains a Russian grammar error; update the
JSON entry for the key "endpoint_pool_exhausted" to the corrected string "Пул
конечных точек исчерпан" (replace "Пул конечная точкаов исчерпан" with the
corrected phrase).
Comment on lines
+585
to
+589
| <div className="flex items-center justify-end gap-2 md:hidden"> | ||
| {canEdit && ( | ||
| <Button variant="outline" className="min-h-[44px] min-w-[44px]" onClick={handleEdit}> | ||
| <Edit className="h-4 w-4" /> | ||
| </Button> |
There was a problem hiding this comment.
移动端编辑按钮缺少可访问性标签
图标按钮建议补充 aria-label（可复用与 actionClone/actionDelete 同组的 i18n key），避免屏幕阅读器无法识别。
建议修改
- <Button variant="outline" className="min-h-[44px] min-w-[44px]" onClick={handleEdit}>
+ <Button
+ variant="outline"
+ className="min-h-[44px] min-w-[44px]"
+ onClick={handleEdit}
+ aria-label={tList("actionEdit")}
+ >📝 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
| <div className="flex items-center justify-end gap-2 md:hidden"> | |
| {canEdit && ( | |
| <Button variant="outline" className="min-h-[44px] min-w-[44px]" onClick={handleEdit}> | |
| <Edit className="h-4 w-4" /> | |
| </Button> | |
| <div className="flex items-center justify-end gap-2 md:hidden"> | |
| {canEdit && ( | |
| <Button | |
| variant="outline" | |
| className="min-h-[44px] min-w-[44px]" | |
| onClick={handleEdit} | |
| aria-label={tList("actionEdit")} | |
| > | |
| <Edit className="h-4 w-4" /> | |
| </Button> |
🤖 Prompt for AI Agents
In `@src/app/`[locale]/settings/providers/_components/provider-rich-list-item.tsx
around lines 585 - 589, The mobile edit icon button lacks an accessibility
label; update the Button JSX in provider-rich-list-item.tsx (the Button that
calls handleEdit) to include an aria-label prop using the same i18n key pattern
used by the actionClone/actionDelete buttons (reuse their translation key for
the "edit" action), so screen readers can announce the control; ensure the
aria-label value comes from the existing i18n lookup rather than a hardcoded
string.
Comment on lines
+214
to
+415
| type ClaudeMetadataUserIdInjectionResult = { | ||
| message: Record<string, unknown>; | ||
| audit: ClaudeMetadataUserIdInjectionSpecialSetting; | ||
| }; | ||
|
|
||
| async function persistSpecialSettings(session: ProxySession): Promise<void> { | ||
| const specialSettings = session.getSpecialSettings(); | ||
| if (!specialSettings || specialSettings.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| if (session.sessionId) { | ||
| await SessionManager.storeSessionSpecialSettings( | ||
| session.sessionId, | ||
| specialSettings, | ||
| session.requestSequence | ||
| ).catch((err) => { | ||
| logger.error("[ProxyForwarder] Failed to store special settings", { | ||
| error: err, | ||
| sessionId: session.sessionId, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| if (session.messageContext?.id) { | ||
| await updateMessageRequestDetails(session.messageContext.id, { | ||
| specialSettings, | ||
| }).catch((err) => { | ||
| logger.error("[ProxyForwarder] Failed to persist special settings", { | ||
| error: err, | ||
| messageRequestId: session.messageContext?.id, | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 为 Claude 请求注入 metadata.user_id | ||
| * | ||
| * 格式：user_{stableHash}_account__session_{sessionId} | ||
| * - stableHash: 基于 API Key ID 生成的稳定哈希（64位 hex），生成后保持不变 | ||
| * - sessionId: 当前请求的 session ID | ||
| * | ||
| * 注意：如果请求体中已存在 metadata.user_id，则保持原样不修改 | ||
| * @internal | ||
| */ | ||
| export function injectClaudeMetadataUserId( | ||
| message: Record<string, unknown>, | ||
| session: ProxySession | ||
| ): Record<string, unknown> { | ||
| const existingMetadata = | ||
| typeof message.metadata === "object" && message.metadata !== null | ||
| ? (message.metadata as Record<string, unknown>) | ||
| : undefined; | ||
|
|
||
| // 检查是否已存在 metadata.user_id | ||
| if (existingMetadata?.user_id !== undefined && existingMetadata?.user_id !== null) { | ||
| return message; | ||
| } | ||
|
|
||
| // 获取必要信息 | ||
| const keyId = session.authState?.key?.id; | ||
| const sessionId = session.sessionId; | ||
|
|
||
| if (keyId == null || !sessionId) { | ||
| return message; | ||
| } | ||
|
|
||
| // 生成稳定的 user hash（基于 API Key ID） | ||
| const stableHash = crypto.createHash("sha256").update(`claude_user_${keyId}`).digest("hex"); | ||
|
|
||
| // 构建 user_id | ||
| const userId = `user_${stableHash}_account__session_${sessionId}`; | ||
|
|
||
| // 注入 metadata | ||
| const newMetadata = { | ||
| ...existingMetadata, | ||
| user_id: userId, | ||
| }; | ||
|
|
||
| return { | ||
| ...message, | ||
| metadata: newMetadata, | ||
| }; | ||
| } | ||
|
|
||
| function applyClaudeMetadataUserIdInjectionWithAudit( | ||
| message: Record<string, unknown>, | ||
| session: ProxySession, | ||
| enabled: boolean | ||
| ): ClaudeMetadataUserIdInjectionResult | null { | ||
| const keyId = session.authState?.key?.id ?? null; | ||
| const sessionId = session.sessionId ?? null; | ||
|
|
||
| if (!enabled) { | ||
| logger.info("[ProxyForwarder] Claude metadata.user_id injection skipped", { | ||
| enabled, | ||
| hit: false, | ||
| reason: "disabled", | ||
| keyId, | ||
| sessionId, | ||
| }); | ||
| return null; | ||
| } | ||
|
|
||
| const existingMetadata = | ||
| typeof message.metadata === "object" && message.metadata !== null | ||
| ? (message.metadata as Record<string, unknown>) | ||
| : undefined; | ||
|
|
||
| if (existingMetadata?.user_id !== undefined && existingMetadata?.user_id !== null) { | ||
| logger.info("[ProxyForwarder] Claude metadata.user_id injection skipped", { | ||
| enabled, | ||
| hit: false, | ||
| reason: "already_exists", | ||
| keyId, | ||
| sessionId, | ||
| }); | ||
|
|
||
| return { | ||
| message, | ||
| audit: { | ||
| type: "claude_metadata_user_id_injection", | ||
| scope: "request", | ||
| hit: false, | ||
| action: "skipped", | ||
| reason: "already_exists", | ||
| keyId, | ||
| sessionId, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| if (keyId == null) { | ||
| logger.info("[ProxyForwarder] Claude metadata.user_id injection skipped", { | ||
| enabled, | ||
| hit: false, | ||
| reason: "missing_key_id", | ||
| keyId, | ||
| sessionId, | ||
| }); | ||
|
|
||
| return { | ||
| message, | ||
| audit: { | ||
| type: "claude_metadata_user_id_injection", | ||
| scope: "request", | ||
| hit: false, | ||
| action: "skipped", | ||
| reason: "missing_key_id", | ||
| keyId, | ||
| sessionId, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| if (!sessionId) { | ||
| logger.info("[ProxyForwarder] Claude metadata.user_id injection skipped", { | ||
| enabled, | ||
| hit: false, | ||
| reason: "missing_session_id", | ||
| keyId, | ||
| sessionId, | ||
| }); | ||
|
|
||
| return { | ||
| message, | ||
| audit: { | ||
| type: "claude_metadata_user_id_injection", | ||
| scope: "request", | ||
| hit: false, | ||
| action: "skipped", | ||
| reason: "missing_session_id", | ||
| keyId, | ||
| sessionId, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| const injectedMessage = injectClaudeMetadataUserId(message, session); | ||
|
|
||
| logger.info("[ProxyForwarder] Claude metadata.user_id injection applied", { | ||
| enabled, | ||
| hit: true, | ||
| reason: "injected", | ||
| keyId, | ||
| sessionId, | ||
| }); | ||
|
|
||
| return { | ||
| message: injectedMessage, | ||
| audit: { | ||
| type: "claude_metadata_user_id_injection", | ||
| scope: "request", | ||
| hit: true, | ||
| action: "injected", | ||
| reason: "injected", | ||
| keyId, | ||
| sessionId, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me check the full context of the forwarder.ts file
wc -l src/app/v1/_lib/proxy/forwarder.tsRepository: ding113/claude-code-hub
Length of output: 106
🏁 Script executed:
# Look for how stableHash/user_id is used after creation
rg "user_id|stableHash|injectClaudeMetadataUserId" src/app/v1/_lib/proxy/forwarder.ts -A 3 -B 1Repository: ding113/claude-code-hub
Length of output: 2700
🏁 Script executed:
# Check if there are any server secrets/environment variables or HMAC patterns used elsewhere
rg "HMAC|hmac|secret|SECRET|pepper|PEPPER" src/ -t ts -t tsx --max-count 20Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Look at the API key security model and if keyId is considered sensitive
rg "keyId|key\.id" src/ -t ts -t tsx -B 2 -A 2 | head -100Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Check how metadata.user_id is transmitted and if it's exposed externally
rg "metadata.*user_id|user_id.*metadata" src/ -t ts -t tsx -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Try searching for secrets management or configuration without type filter
rg "hmac|HMAC|secret|SECRET|pepper" src/app/v1/_lib/proxy/ -A 2 -B 1Repository: ding113/claude-code-hub
Length of output: 330
🏁 Script executed:
# Check the broader security context - look at session and auth state
rg "authState|session\.auth" src/app/v1/_lib/proxy/forwarder.ts -A 3 -B 1 | head -80Repository: ding113/claude-code-hub
Length of output: 745
🏁 Script executed:
# Look for environment variables or config that might contain secrets
rg "process\.env|config\.|getConfig" src/app/v1/_lib/proxy/forwarder.ts -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 927
🏁 Script executed:
# Check if there's any documentation about the security model of keyId
find src -name "*.md" -o -name "*.txt" | xargs grep -l "keyId\|API.*key\|security" 2>/dev/null | head -5Repository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Search for how keyId is created/generated and its scope
rg "keyId.*=" src/ -t ts -B 3 -A 1 | head -60Repository: ding113/claude-code-hub
Length of output: 3112
🏁 Script executed:
# Check if there are any existing crypto utilities or security utilities
find src -type f -name "*crypto*" -o -name "*security*" -o -name "*secret*" -o -name "*hash*"Repository: ding113/claude-code-hub
Length of output: 83
🏁 Script executed:
# Check the security directory
find src/lib/security -type f | head -20Repository: ding113/claude-code-hub
Length of output: 147
🏁 Script executed:
# Look at the security directory contents
ls -la src/lib/security/Repository: ding113/claude-code-hub
Length of output: 331
🏁 Script executed:
# Check if there are any HMAC or crypto utilities in the codebase
rg "createHmac|HMAC" src/ -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 625
🏁 Script executed:
# Check the API key model - understand what keyId represents
rg "type.*Key|interface.*Key|class.*Key" src/ -t ts -A 5 | head -100Repository: ding113/claude-code-hub
Length of output: 5036
🏁 Script executed:
# Check for any existing hashing strategies or similar patterns
rg "stableHash|stable.*hash|hash.*stable" src/ -B 3 -A 3Repository: ding113/claude-code-hub
Length of output: 1191
使用带服务端密钥的 HMAC 来生成用户标识哈希，防止 keyId 枚举攻击
虽然 keyId 在代码库中已被记录和暴露，但当 user_id 被发送到外部服务时，直接使用 SHA256(keyId) 会允许枚举攻击确定正在使用的 API 密钥。建议引入服务端 secret/pepper，使用 createHmac("sha256", secret) 代替 createHash("sha256")，保持稳定性的同时增加计算成本。参考 src/lib/webhook/notifier.ts 中的现有 HMAC 模式。
🤖 Prompt for AI Agents
In `@src/app/v1/_lib/proxy/forwarder.ts` around lines 214 - 415, The current
injectClaudeMetadataUserId function uses crypto.createHash("sha256") on keyId
which allows keyId enumeration; change the stableHash generation to use an HMAC
with a server-side secret (pepper) — e.g., crypto.createHmac("sha256",
serverSecret).update(`claude_user_${keyId}`).digest("hex") — where serverSecret
is read from a secure config/env and remains constant across restarts; update
injectClaudeMetadataUserId (and any callers like
applyClaudeMetadataUserIdInjectionWithAudit) to use that HMAC-based stableHash,
ensure the secret is injected/accessible (e.g., via existing config/provider),
and keep the rest of the user_id format unchanged so downstream behavior stays
stable.
Comment on lines
+2506
to
+2520
| // ⚠️ HTTP 错误：不要在读取响应体之前清除响应超时定时器 | ||
| // 原因：某些上游会在返回 4xx/5xx 后“卡住不结束 body”， | ||
| // 若提前 clearTimeout，会导致 ProxyError.fromUpstreamResponse() 的 response.text() 无限等待， | ||
| // 从而让整条请求链路（含客户端）悬挂，前端表现为一直“请求中”。 | ||
| // | ||
| // 正确策略：保留 response timeout 继续监控 body 读取，并在 finally 里清理定时器。 | ||
| try { | ||
| throw await ProxyError.fromUpstreamResponse(response, { | ||
| id: provider.id, | ||
| name: provider.name, | ||
| }); | ||
| } finally { | ||
| if (responseTimeoutId) { | ||
| clearTimeout(responseTimeoutId); | ||
| } |
There was a problem hiding this comment.
注释中移除 emoji 字符
TS 文件禁止 emoji，建议删除该注释中的符号。
建议修改
- // ⚠️ HTTP 错误：不要在读取响应体之前清除响应超时定时器
+ // HTTP 错误：不要在读取响应体之前清除响应超时定时器📝 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
| // ⚠️ HTTP 错误：不要在读取响应体之前清除响应超时定时器 | |
| // 原因：某些上游会在返回 4xx/5xx 后“卡住不结束 body”， | |
| // 若提前 clearTimeout，会导致 ProxyError.fromUpstreamResponse() 的 response.text() 无限等待， | |
| // 从而让整条请求链路（含客户端）悬挂，前端表现为一直“请求中”。 | |
| // | |
| // 正确策略：保留 response timeout 继续监控 body 读取，并在 finally 里清理定时器。 | |
| try { | |
| throw await ProxyError.fromUpstreamResponse(response, { | |
| id: provider.id, | |
| name: provider.name, | |
| }); | |
| } finally { | |
| if (responseTimeoutId) { | |
| clearTimeout(responseTimeoutId); | |
| } | |
| // HTTP 错误：不要在读取响应体之前清除响应超时定时器 | |
| // 原因：某些上游会在返回 4xx/5xx 后"卡住不结束 body"， | |
| // 若提前 clearTimeout，会导致 ProxyError.fromUpstreamResponse() 的 response.text() 无限等待， | |
| // 从而让整条请求链路（含客户端）悬挂，前端表现为一直"请求中"。 | |
| // | |
| // 正确策略：保留 response timeout 继续监控 body 读取，并在 finally 里清理定时器。 | |
| try { | |
| throw await ProxyError.fromUpstreamResponse(response, { | |
| id: provider.id, | |
| name: provider.name, | |
| }); | |
| } finally { | |
| if (responseTimeoutId) { | |
| clearTimeout(responseTimeoutId); | |
| } |
🤖 Prompt for AI Agents
In `@src/app/v1/_lib/proxy/forwarder.ts` around lines 2506 - 2520, Remove the
emoji characters from the comment block above the try/finally so the file
conforms to the TS no-emoji rule; keep the comment text and intent intact
(explain not to clear responseTimeoutId before reading response body and to
clear it in the finally block), and ensure related identifiers
(ProxyError.fromUpstreamResponse, response, responseTimeoutId, and the finally
cleanup) remain unchanged so behavior is unaffected.
| import type { GeminiResponse } from "../gemini/types"; | ||
| import { isClientAbortError } from "./errors"; | ||
| import type { ProxySession } from "./session"; | ||
| import { consumeDeferredStreamingFinalization } from "./stream-finalization"; |
There was a problem hiding this comment.
使用 @/ 路径别名导入本地模块
这里新增的相对路径导入会绕过仓库约定的别名路径。建议改为 @/app/v1/_lib/proxy/stream-finalization 以保持一致性。
建议修改
-import { consumeDeferredStreamingFinalization } from "./stream-finalization";
+import { consumeDeferredStreamingFinalization } from "@/app/v1/_lib/proxy/stream-finalization";As per coding guidelines: Use path alias @/ to reference files in ./src/ directory
📝 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 { consumeDeferredStreamingFinalization } from "./stream-finalization"; | |
| import { consumeDeferredStreamingFinalization } from "@/app/v1/_lib/proxy/stream-finalization"; |
🤖 Prompt for AI Agents
In `@src/app/v1/_lib/proxy/response-handler.ts` at line 27, The import of
consumeDeferredStreamingFinalization currently uses a relative path
("./stream-finalization"); replace it with the repository path-alias import
'@/app/v1/_lib/proxy/stream-finalization' to follow the project convention.
Locate the import statement that brings in consumeDeferredStreamingFinalization
and update the module specifier to the alias form so all local module imports
consistently use '@/...' instead of relative paths.
Comment on lines
+56
to
+86
| export interface EndpointFilterStats { | ||
| total: number; | ||
| enabled: number; | ||
| circuitOpen: number; | ||
| available: number; | ||
| } | ||
|
|
||
| /** | ||
| * Collect endpoint filter statistics for a given vendor/type. | ||
| * | ||
| * Used for audit trail when all endpoints are exhausted (strict block). | ||
| * Returns null only when the raw endpoint query itself fails. | ||
| */ | ||
| export async function getEndpointFilterStats(input: { | ||
| vendorId: number; | ||
| providerType: ProviderType; | ||
| }): Promise<EndpointFilterStats> { | ||
| const endpoints = await findProviderEndpointsByVendorAndType(input.vendorId, input.providerType); | ||
| const total = endpoints.length; | ||
| const enabled = endpoints.filter((e) => e.isEnabled && !e.deletedAt).length; | ||
|
|
||
| const circuitResults = await Promise.all( | ||
| endpoints | ||
| .filter((e) => e.isEnabled && !e.deletedAt) | ||
| .map(async (e) => isEndpointCircuitOpen(e.id)) | ||
| ); | ||
| const circuitOpen = circuitResults.filter(Boolean).length; | ||
| const available = enabled - circuitOpen; | ||
|
|
||
| return { total, enabled, circuitOpen, available }; | ||
| } |
There was a problem hiding this comment.
注释与返回类型不一致。
Line 67 写明“返回 null”，但实现与签名不会返回 null；建议改注释为会抛错，或调整签名并捕获异常。
修正注释以匹配实现
- * Returns null only when the raw endpoint query itself fails.
+ * Throws if the raw endpoint query itself fails.🤖 Prompt for AI Agents
In `@src/lib/provider-endpoints/endpoint-selector.ts` around lines 56 - 86, The
JSDoc for getEndpointFilterStats incorrectly states it "Returns null" on raw
query failure while the implementation and signature always return
EndpointFilterStats; either update the comment to say the function will throw if
findProviderEndpointsByVendorAndType fails (and mention it queries
findProviderEndpointsByVendorAndType and uses isEndpointCircuitOpen), or change
the signature to Promise<EndpointFilterStats | null> and wrap the call to
findProviderEndpointsByVendorAndType in a try/catch that returns null on error;
make the corresponding adjustment to the EndpointFilterStats usage and tests
accordingly.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const redisUrl = process.env.REDIS_URL; | ||
| const isEnabled = process.env.ENABLE_RATE_LIMIT === "true"; | ||
| const rateLimitRaw = process.env.ENABLE_RATE_LIMIT?.trim(); | ||
| const isEnabled = rateLimitRaw !== "false" && rateLimitRaw !== "0"; |
Contributor
There was a problem hiding this comment.
[High] [STANDARD-VIOLATION] ENABLE_RATE_LIMIT default behavior changed from opt-in to opt-out
Why this is a problem: The old logic was process.env.ENABLE_RATE_LIMIT === "true" (Redis/rate-limiting disabled by default). The new logic is rateLimitRaw \!== "false" && rateLimitRaw \!== "0" (enabled by default). This means any deployment that previously relied on the default (unset) behavior to keep rate limiting disabled will now have it silently enabled. If REDIS_URL is not configured, this still returns null safely, but if REDIS_URL IS set without explicitly setting ENABLE_RATE_LIMIT, Redis connections will now be established where they were not before.
This is a breaking change in configuration semantics that should be documented in migration notes.
Suggested fix:
// Option A: Keep opt-in semantics (backward compatible)
const isEnabled = rateLimitRaw === "true" || rateLimitRaw === "1";
// Option B: If opt-out is intentional, document it clearly in .env.example
// and add a migration note in the PR description / CHANGELOG
| // 将新建 key 写入 Vacuum Filter（提升新 key 的即时可用性；失败不影响正确性） | ||
| try { | ||
| apiKeyVacuumFilter.noteExistingKey(created.key); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
[Medium] [ERROR-SILENT] Empty catch block swallows vacuum filter errors without logging
Why this is a problem: If apiKeyVacuumFilter.noteExistingKey() throws, the error is silently swallowed with only a // ignore comment. While the comment explains intent ("best effort"), there is zero observability into failures. If the vacuum filter consistently fails to register new keys, operators have no way to detect this through logs or monitoring, potentially leading to intermittent 401 rejections for newly created keys.
Suggested fix:
try {
apiKeyVacuumFilter.noteExistingKey(created.key);
} catch (error) {
logger.debug("[KeyRepository] Failed to register key in vacuum filter", {
error: error instanceof Error ? error.message : String(error),
});
}
| const redisBestEffortTimeoutMs = 200; | ||
| const redisTasks: Array<Promise<unknown>> = []; | ||
|
|
||
| redisTasks.push(cacheActiveKey(created).catch(() => {})); |
Contributor
There was a problem hiding this comment.
[Medium] [ERROR-SILENT] Systematic silent .catch(() => {}) across all Redis cache operations
Why this is a problem: This file has 9 instances of .catch(() => {}) on Redis cache operations (lines 194, 199, 280, 282, 449, 509, 564, 569, 691). While each is documented as "best effort" and correctness is not affected (DB fallback exists), the complete absence of logging means:
- Redis connection failures are invisible to operators
- Cache layer degradation cannot be detected through monitoring
- Debugging intermittent auth issues becomes significantly harder
A single shared helper would reduce noise while adding observability.
Suggested fix:
// Add a shared helper at the top of the file:
function logCacheError(action: string, error: unknown): void {
logger.debug("[KeyRepository] Cache operation failed", {
action,
error: error instanceof Error ? error.message : String(error),
});
}
// Then replace .catch(() => {}) with:
cacheActiveKey(created).catch((e) => logCacheError("cacheActiveKey", e));
publishCacheInvalidation(CHANNEL_API_KEYS_UPDATED).catch((e) => logCacheError("publishInvalidation", e));
// ... etc
Contributor
There was a problem hiding this comment.
Code Review Summary
This is a large release PR (v0.5.5) covering proxy reliability improvements (fake-200 detection, stuck-request prevention, circuit-breaker recovery), provider management UI overhaul, auth pipeline hardening with a new Redis cache + vacuum filter layer, and several new features. The architecture of the deferred streaming finalization and the multi-layer auth cache is well-designed. Three issues were identified that warrant attention before merge.
PR Size: XL
- Lines changed: 28,069 (27,314 + 755)
- Files changed: 189
Split suggestion: Given the XL size, future releases could benefit from splitting proxy reliability fixes, auth pipeline hardening, and UI changes into separate PRs for easier review and safer rollback.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 2 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Standards | 0 | 1 | 0 | 0 |
High Priority Issues (Should Fix)
ENABLE_RATE_LIMITdefault behavior change (src/lib/redis/client.ts:84): Logic changed from opt-in (=== "true") to opt-out (!== "false"), silently enabling Redis connections for deployments that previously relied on the unset default. This is a breaking configuration change that should be documented in migration notes.
Medium Priority Issues (Consider Fixing)
-
Empty catch block in
createKey(src/repository/key.ts:184): Vacuum filter registration error is swallowed withcatch { // ignore }and no logging, reducing observability. -
Systematic silent
.catch(() => {})pattern (src/repository/key.ts): 9 instances of Redis cache errors silently swallowed across the file. A shared debug-level logging helper would add observability without impacting performance.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Claude AI
| // Explicitly suppress rejections to avoid double toast | ||
| queryClient | ||
| .invalidateQueries({ queryKey: ["provider-endpoints", vendorId] }) | ||
| .catch(() => undefined); |
Contributor
There was a problem hiding this comment.
[CRITICAL] [ERROR-SILENT] Promise rejection is swallowed during cache invalidation
Evidence (src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:514):
.catch(() => undefined);Violates: "No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect."
Why this is a problem: If invalidateQueries() rejects, the UI can stay stale with no log/telemetry, making this failure mode hard to debug.
Suggested fix:
queryClient
.invalidateQueries({ queryKey: ["provider-endpoints", vendorId] })
.catch((err) => {
console.error("[AddEndpointButton] invalidateQueries failed", err);
});(Apply the same pattern to the fixedProviderType invalidation call.)
|
|
||
| const parsed = JSON.parse(raw) as CachedKeyPayloadV1; | ||
| if (parsed?.v !== 1 || !parsed.key) { | ||
| redis.del(redisKey).catch(() => {}); |
Contributor
There was a problem hiding this comment.
[CRITICAL] [ERROR-SWALLOWED] Redis cache delete failures are silently ignored
Evidence (src/lib/security/api-key-auth-cache.ts:217):
redis.del(redisKey).catch(() => {});Violates: "No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect."
Why this is a problem: This is auth-adjacent cache hygiene. If del() fails (e.g., WRONGTYPE, transient Redis errors), invalid/stale entries can persist with no signal, making auth/cache incidents hard to diagnose and potentially extending stale reads.
Suggested fix (best-effort delete with logging):
void redis.del(redisKey).catch((error) => {
logger.debug(
{ error: error instanceof Error ? error.message : String(error), redisKey },
"[ApiKeyAuthCache] Failed to delete cache entry"
);
});(Apply the same pattern to the other redis.del(...).catch(() => {}) and replace catch { // ignore } blocks with catch (error) { logger.debug(...) }.)
| const redisBestEffortTimeoutMs = 200; | ||
| const redisTasks: Array<Promise<unknown>> = []; | ||
|
|
||
| redisTasks.push(cacheActiveKey(created).catch(() => {})); |
Contributor
There was a problem hiding this comment.
[CRITICAL] [ERROR-SILENT] Best-effort auth cache/broadcast work swallows errors without logging
Evidence (src/repository/key.ts:194):
redisTasks.push(cacheActiveKey(created).catch(() => {}));Also includes a silent catch { /* ignore */ } around apiKeyVacuumFilter.noteExistingKey(created.key).
Violates: "No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect."
Why this is a problem: This runs on API key creation/update. If cache write/broadcast/filter updates fail, you lose visibility into multi-instance consistency issues and can prolong stale cache windows. Silent failures here are security/ops blind spots.
Suggested fix (log without leaking the key secret):
import { logger } from "@/lib/logger";
try {
apiKeyVacuumFilter.noteExistingKey(created.key);
} catch (error) {
logger.warn("[KeyRepository] apiKeyVacuumFilter.noteExistingKey failed", {
error: error instanceof Error ? error.message : String(error),
keyId: created.id,
});
}
redisTasks.push(
cacheActiveKey(created).catch((error) => {
logger.debug(
{ error: error instanceof Error ? error.message : String(error), keyId: created.id },
"[KeyRepository] cacheActiveKey failed"
);
})
);(Apply the same catch((error) => logger...) pattern to other .catch(() => {}) call sites in this file.)
| // HTTP 错误：清除响应超时定时器 | ||
| if (responseTimeoutId) { | ||
| clearTimeout(responseTimeoutId); | ||
| // ⚠️ HTTP 错误：不要在读取响应体之前清除响应超时定时器 |
Contributor
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Emoji character introduced in code comment
Evidence (src/app/v1/_lib/proxy/forwarder.ts:2506):
// ⚠️ HTTP 错误：不要在读取响应体之前清除响应超时定时器Violates CLAUDE.md: "No Emoji in Code - Never use emoji characters in any code, comments, or string literals"
Why this is a problem: This breaks a repo-wide standard and makes comment rendering/grepping inconsistent.
Suggested fix:
// WARNING: HTTP 错误：不要在读取响应体之前清除响应超时定时器
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR is XL and touches security-sensitive auth paths (API key caching/vacuum filter), core proxy forwarding/stream handling, provider endpoint management, and multiple DB/i18n changes. I found two must-fix security issues where new code silently swallows cache/invalidation errors (no logging), plus a UI silent-failure pattern and a repo-standard violation.
PR Size: XL
- Lines changed: 27974
- Files changed: 176
- Split suggestions (required for XL): Split into (1) auth cache + vacuum filter + repo integration + tests, (2) proxy forwarder/response-handler/stream finalization + tests, (3) provider endpoints (selector/repository/UI/i18n) + tests, (4) leaderboard changes + tests, (5) migrations/docs.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 2 | 0 | 0 | 0 |
| Error Handling | 1 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 1 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
src/lib/security/api-key-auth-cache.ts:217- silent Redisdel()failures (.catch(() => {})/catch { /* ignore */ }) in auth cache hygiene (Confidence: 98)src/repository/key.ts:194- silent failures around vacuum filter + Redis cache/broadcast tasks on key lifecycle (Confidence: 98)
High Priority Issues (Should Fix)
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:514-.catch(() => undefined)suppressesinvalidateQueries()rejections with no logging (Confidence: 80)src/app/v1/_lib/proxy/forwarder.ts:2506- emoji introduced in comment violates repo standard (Confidence: 80)
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.
Findings (Diff-Only, Ordered By Severity)
-
src/lib/security/api-key-auth-cache.ts:217— [CRITICAL] [ERROR-SWALLOWED] silent cache delete failures- Evidence:
redis.del(redisKey).catch(() => {}); - Violates:
No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect. - Suggested fix:
void redis.del(redisKey).catch((error) => { logger.debug( { error: error instanceof Error ? error.message : String(error), redisKey }, "[ApiKeyAuthCache] Failed to delete cache entry" ); });
- Evidence:
-
src/repository/key.ts:194— [CRITICAL] [ERROR-SILENT] best-effort auth cache/broadcast work swallows errors without logging- Evidence:
redisTasks.push(cacheActiveKey(created).catch(() => {}));andcatch { /* ignore */ }aroundapiKeyVacuumFilter.noteExistingKey(...) - Violates:
No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect. - Suggested fix:
import { logger } from "@/lib/logger"; try { apiKeyVacuumFilter.noteExistingKey(created.key); } catch (error) { logger.warn("[KeyRepository] apiKeyVacuumFilter.noteExistingKey failed", { error: error instanceof Error ? error.message : String(error), keyId: created.id, }); } redisTasks.push( cacheActiveKey(created).catch((error) => { logger.debug( { error: error instanceof Error ? error.message : String(error), keyId: created.id }, "[KeyRepository] cacheActiveKey failed" ); }) );
- Evidence:
-
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:514— [CRITICAL] [ERROR-SILENT] promise rejection swallowed during cache invalidation- Evidence:
.catch(() => undefined); - Violates:
No Silent Failures: Any error caught without logging or user feedback is a CRITICAL defect. - Suggested fix:
queryClient .invalidateQueries({ queryKey: ["provider-endpoints", vendorId] }) .catch((err) => { console.error("[AddEndpointButton] invalidateQueries failed", err); });
- Evidence:
-
src/app/v1/_lib/proxy/forwarder.ts:2506— [MEDIUM] [STANDARD-VIOLATION] emoji introduced in code comment- Evidence:
// ⚠️ HTTP 错误：不要在读取响应体之前清除响应超时定时器 - Violates CLAUDE.md:
No Emoji in Code - Never use emoji characters in any code, comments, or string literals - Suggested fix:
// WARNING: HTTP 错误：不要在读取响应体之前清除响应超时定时器
- Evidence:
Actions Completed
- Applied PR label:
size/XL - Posted inline review comments for the issues above
- Submitted PR review summary via
gh pr review
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
4 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
Release v0.5.5 — a major reliability and UX update focused on proxy stability (fake-200 detection, stuck-request prevention, proper circuit-breaker recovery), provider management overhaul (per-group priority, inline editing, endpoint circuit visibility), and performance improvements (vacuum filter hot-path optimization, auth pipeline hardening).
Issues Closed
Closes #682 — Remember column visibility settings in logs view
Closes #697 — Vendor-type circuit breaker incorrectly disabling all keys
Closes #728 — View circuit breaker reasons directly in provider management UI
Closes #741 — Provider priority not working correctly
Closes #742 — Endpoint updates not persisting / causing other endpoints to disappear
Closes #749 — Fake HTTP 200 responses incorrectly recorded as successful
Closes #752 — Vacuum filter hot-path performance regression vs Set.has
Closes #754 — Endpoint circuit breaker lacking visibility in UI and decision chain
Closes #760 — Provider error logic and display issues (503 errors, priority-based failover, timeout logging)
Changes
Proxy Reliability
response-handler.ts,forwarder.ts)forwarder.ts,stream-finalization.ts)provider-selector.ts)session.ts)Provider Management UI
priority-edit-popover.tsx,group-edit-combobox.tsx,routing-section.tsx)endpoint-status.ts,provider-endpoints-table.tsx,provider-rich-list-item.tsx)inline-edit-popover.tsx)provider-chain-popover.tsx,SummaryTab.tsx)Security & Auth
auth.ts)metadata.user_idfor relay provider cache support, with system-level toggle (provider-overrides.ts,system-settings-form.tsx)Performance
instrumentation.ts)Anthropic Features
thinking-budget-rectifier.ts,provider-overrides.ts)Leaderboard
leaderboard-table.tsx,leaderboard-view.tsx,leaderboard/route.ts)Infrastructure
copy-version-to-standalone.cjs)Database Migrations
4 new migrations (0063-0066):
enable_claude_metadata_user_id_injectiontosystem_settingsgroup_priorities(JSONB) toprovidersprovider_endpointswith soft-delete filteranthropic_adaptive_thinking(JSONB) toproviders; migrate legacyadaptivevalues toinheritTesting
i18n
All 5 languages updated (zh-CN, zh-TW, en, ja, ru) for new provider management strings, circuit visibility labels, leaderboard metrics, and adaptive thinking settings.
Description enhanced by Claude AI