Conversation
修复 PostgreSQL 查询中的日期计算 bug,当 maxAgeDays = 36500(100年)时, 使用 Date.setDate() 会导致日期回退到 1926-03-10,引发数据库错误。 **问题根因**: - JavaScript Date.setDate() 在处理超大天数减法时会产生不合理的历史日期 - 查询参数 1926-03-10 导致 PostgreSQL 共享内存段调整失败 **修复方案**: - 使用时间戳计算:`new Date(Date.now() - days * 24 * 60 * 60 * 1000)` - 替代原有的 `setDate(getDate() - days)` 方式 - 适用于任意大的天数值(包括 36500 天 = 100 年) **影响范围**: - src/repository/statistics.ts: - sumKeyTotalCost() - Key 历史总消费查询 - sumUserTotalCost() - 用户历史总消费查询 **测试覆盖**: - 新增单元测试验证 36500 天和 100000 天的场景 - 验证日期计算结果在合理范围内(非 1926 年) - 全量测试通过(262 文件,2245 测试) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: Key/User 并发 Session 原子化校验与追踪 * 并发 Session:补齐 global 清理并移除非原子降级 - Lua:增加 global:active_sessions 的过期清理,避免长时间流量下膨胀\n- RateLimitGuard:sessionId 缺失时兜底生成,并继续走原子 check+track,避免回退到非原子路径\n- Tests:补充 sessionId 缺失用例;稳定 endpoint-circuit-breaker 的动态 import mock * RateLimitGuard:sessionId 缺失时记录告警 - 当 sessionId 在 rate-limit-guard 阶段缺失时记录 warn 日志,便于定位异常链路\n- 提示可能存在原子性缺口(SessionGuard 未按预期分配 sessionId) * fix(rate-limit): 统一 active_sessions key 并修复空 sessionId 兜底 * fix(session): terminate 时清理 user active_sessions * fix(rate-limit): 禁止 key ZSET 绕过 user 并发上限 * 修复并发会话限制:补齐清理与错误码 - resetUserAllStatistics 同步清理 user 级 active_sessions\n- checkAndTrackKeyUserSession 改为返回 reasonCode/reasonParams(便于 i18n)\n- 更新相关单元测试与脚本导入路径 * refactor(concurrent): 统一 Key/User 并发上限解析 * docs(concurrent): 补齐导出函数注释并去除测试硬编码 key * docs(rate-limit): 补齐守卫与测试辅助函数注释 * docs(session): 补齐 SessionManager header 解析注释 * docs: 补齐限流服务与批量更新错误注释 * docs(users): 补齐批量接口类型注释 --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com>
…r from system prompt (#784) Claude Code client v2.1.36+ injects x-anthropic-billing-header as a text block inside the request body's system content array. Non-native Anthropic upstreams (e.g. Amazon Bedrock) reject this with 400. This proactive rectifier strips these blocks before forwarding, with a system settings toggle (default: enabled). - New rectifier module with regex-based detection - Full backend wiring: schema, repository, cache, validation, action - UI toggle in system settings form with i18n (5 locales) - SpecialSetting audit trail for removed blocks - 14 unit tests covering all edge cases - Drizzle migration for new boolean column
The previous fix (67c2940) incorrectly diagnosed the issue as a Date.setDate() overflow. In reality, 36500 days ago IS 1926 — both setDate() and timestamp subtraction produce the same result. The actual problem was that querying a 100-year date range caused PostgreSQL to exhaust shared memory on resource-constrained instances. Change ALL_TIME_MAX_AGE_DAYS from 36500 to Infinity, and make sumKeyTotalCost/sumUserTotalCost conditionally omit the created_at filter when maxAgeDays is non-finite, zero, or negative.
The previous fix removed the date filter but the page still crashed PostgreSQL shared memory. Root cause: N+M concurrent SUM(cost_usd) queries (one per user + one per key) via Promise.all. Add sumUserTotalCostBatch and sumKeyTotalCostBatchByIds that use GROUP BY to aggregate all users/keys in a single SQL query each. Rewrite quotas/users page to use 2 batch queries instead of N+M individual ones.
📝 Walkthrough工作流概述本PR添加了账单标头整流器功能,移除Claude Code客户端注入的x-anthropic-billing-header文本块,以防止非原生Anthropic上游返回400错误。同时引入数据库迁移、系统设置配置、代理转发逻辑、会话跟踪改进和批量统计查询优化。 变更
评估代码审查工作量🎯 4 (复杂) | ⏱️ ~60 分钟 可能相关的PR
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new billing header rectification feature to improve compatibility with Anthropic upstreams, alongside significant performance enhancements for the user quota dashboard through batch database queries. It also strengthens concurrent session limit enforcement with atomic Redis operations and standardizes Redis key management for better Redis Cluster compatibility. Additionally, memory efficiency for handling large streaming responses has been improved, and all-time usage queries now provide more accurate historical data. 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
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review
This is an excellent release pull request that not only introduces the new "Billing Header Rectifier" feature but also includes significant improvements to performance, correctness, and robustness across the application. The implementation of atomic session tracking via a Lua script is a standout change, effectively resolving a critical race condition in concurrent session limiting. Furthermore, the performance optimization on the user quotas page, which replaces N+M queries with a few batch operations, is a major improvement. Other refactorings, such as centralizing Redis key generation with cluster support and making the agent pool shutdown more resilient, demonstrate great attention to detail and system stability. The code quality is high, and the changes are well-executed. Great work on this release.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/repository/system-config.ts (1)
402-435:⚠️ Potential issue | 🔴 Critical
returning子句中缺少enableBillingHeaderRectifier字段。更新操作的
.returning()中包含了enableThinkingBudgetRectifier(第 422 行)后直接跳到了enableCodexSessionIdCompletion(第 423 行),遗漏了enableBillingHeaderRectifier。该字段已在回退设置(第 154 行)、select 查询(第 198 行)和更新处理(第 355-356 行)中定义,但在返回子句中缺失,导致更新后返回的对象中该字段为undefined。🔧 建议修复
enableThinkingBudgetRectifier: systemSettings.enableThinkingBudgetRectifier, + enableBillingHeaderRectifier: systemSettings.enableBillingHeaderRectifier, enableCodexSessionIdCompletion: systemSettings.enableCodexSessionIdCompletion,
🤖 Fix all issues with AI agents
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 1156-1174: The current byte-wise split using value.subarray(0,
remainingHeadBytes) can cut a multibyte UTF‑8 character; change the split to a
safe UTF‑8 boundary: before slicing, scan backward from remainingHeadBytes up to
3 bytes to find a byte where (byte & 0xC0) != 0x80 (a UTF‑8 start byte) and use
that index as splitIndex; if none found, set splitIndex = remainingHeadBytes
(fallback). Replace uses of value.subarray(0, remainingHeadBytes) and
value.subarray(remainingHeadBytes) with value.subarray(0, splitIndex) and
value.subarray(splitIndex) and keep decoder.decode(..., { stream: true }) and
pushChunk calls the same (adjust counts passed to pushChunk to reflect
splitIndex instead of remainingHeadBytes). Ensure the helper logic is used in
the branch that references inTailMode, headBufferedBytes, MAX_STATS_HEAD_BYTES,
decoder, pushChunk, value, and chunkSize.
🧹 Nitpick comments (6)
src/lib/proxy-agent/agent-pool.ts (1)
385-398: 建议:将destroy优先于close的策略选择记录为更显眼的注释或常量。当前的中文注释已解释了为何优先使用
destroy(close()可能等待 in-flight 请求导致长期阻塞)。这是一个关键的设计决策。可以考虑在类级别或方法签名处用英文 JSDoc 简要说明这一策略,方便不同语言背景的开发者理解。此外,如果未来需要支持"等待 in-flight 请求完成后再关闭"的场景(例如优雅关闭时),可能需要区分两种关闭模式。当前的实现对 eviction 场景是正确的。
src/app/v1/_lib/proxy/response-handler.ts (1)
1008-1038:pushToTail在每次pushChunk调用时都会重新创建闭包。
pushToTail定义在pushChunk内部,意味着每次调用pushChunk(即每接收到一个流 chunk)都会创建一个新的函数对象。对于高吞吐的流式响应场景(可能上千个 chunk),可以将pushToTail提升到与pushChunk同一作用域以避免不必要的分配。现代 V8 引擎对此有较好的优化,实际影响有限,可作为后续优化项。
将 pushToTail 提升到外层作用域
+ const pushToTail = (text: string, bytes: number) => { + tailChunks.push(text); + tailChunkBytes.push(bytes); + tailBufferedBytes += bytes; + + while (tailBufferedBytes > MAX_STATS_TAIL_BYTES && tailHead < tailChunkBytes.length) { + tailBufferedBytes -= tailChunkBytes[tailHead] ?? 0; + tailChunks[tailHead] = ""; + tailChunkBytes[tailHead] = 0; + tailHead += 1; + wasTruncated = true; + } + + if (tailHead > 4096) { + tailChunks.splice(0, tailHead); + tailChunkBytes.splice(0, tailHead); + tailHead = 0; + } + + const keptCount = tailChunks.length - tailHead; + if (keptCount > MAX_STATS_TAIL_CHUNKS) { + const joined = joinTailChunks(); + tailChunks.length = 0; + tailChunkBytes.length = 0; + tailHead = 0; + tailChunks.push(joined); + tailChunkBytes.push(tailBufferedBytes); + } + }; + const pushChunk = (text: string, bytes: number) => { if (!text) return; - const pushToTail = () => { - tailChunks.push(text); - tailChunkBytes.push(bytes); - // ... rest of pushToTail - }; if (!inTailMode) { if (headBufferedBytes + bytes <= MAX_STATS_HEAD_BYTES) { headChunks.push(text); headBufferedBytes += bytes; return; } inTailMode = true; } - pushToTail(); + pushToTail(text, bytes); };src/repository/statistics.ts (1)
819-845:sumUserTotalCostBatch批量查询实现合理。先零初始化所有请求的 userId,再用查询结果覆盖,确保无记录的用户也会返回 0。需注意当
userIds数组非常大时,inArray生成的 SQLIN (...)子句可能影响性能。如果未来有超大批量场景,可考虑分批查询。src/lib/utils/special-settings.ts (1)
100-101: 去重键未包含extractedValues字段。
buildSettingKey仅序列化了[type, hit, removedCount],遗漏了extractedValues。若同一请求中出现两条removedCount相同但extractedValues不同的审计记录,它们会被误判为重复而丢弃。其他类似 case(如
thinking_signature_rectifier)均将主要字段纳入了 key 计算。建议补齐:建议修改
case "billing_header_rectifier": - return JSON.stringify([setting.type, setting.hit, setting.removedCount]); + return JSON.stringify([setting.type, setting.hit, setting.removedCount, setting.extractedValues]);tests/unit/repository/statistics-total-cost.test.ts (1)
13-36: 条件断言可以更严格,但当前实现可接受。注释提到 "should not contain a date filter",但 Line 35 实际只断言了
capturedConditions不为undefined,没有验证条件数量或是否缺少日期过滤器。考虑到 Drizzle ORM 内部条件对象结构较复杂,当前做法是务实的,但如果希望提高测试置信度,可以对where调用参数做更细粒度的快照比较。src/lib/rate-limit/service.ts (1)
527-558: 对checkSessionLimit方法进行弃用或移除,因为该方法已由原子化的checkAndTrackKeyUserSession完全取代。搜索结果确认该方法在整个代码库中没有任何调用方。
checkAndTrackKeyUserSession相比之下提供了更安全的原子性操作,使用 Lua 脚本完成"检查 + 追踪",并能区分新会话与已存在会话的处理。建议尽快标记为@deprecated或直接移除,避免维护两套非原子化的会话检查路径。
|
|
||
| // 尽量填满 head:边界 chunk 可能跨过 head 上限,按 byte 切分以避免 head 少于 1MB | ||
| if (!inTailMode && headBufferedBytes < MAX_STATS_HEAD_BYTES) { | ||
| const remainingHeadBytes = MAX_STATS_HEAD_BYTES - headBufferedBytes; | ||
| if (remainingHeadBytes > 0 && chunkSize > remainingHeadBytes) { | ||
| const headPart = value.subarray(0, remainingHeadBytes); | ||
| const tailPart = value.subarray(remainingHeadBytes); | ||
|
|
||
| const headText = decoder.decode(headPart, { stream: true }); | ||
| pushChunk(headText, remainingHeadBytes); | ||
|
|
||
| const tailText = decoder.decode(tailPart, { stream: true }); | ||
| pushChunk(tailText, chunkSize - remainingHeadBytes); | ||
| } else { | ||
| pushChunk(decoder.decode(value, { stream: true }), chunkSize); | ||
| } | ||
| } else { | ||
| pushChunk(decoder.decode(value, { stream: true }), chunkSize); | ||
| } |
There was a problem hiding this comment.
在任意字节边界切分 Uint8Array 可能截断多字节 UTF-8 字符。
value.subarray(0, remainingHeadBytes) 按字节偏移量切分,如果恰好在一个多字节 UTF-8 字符中间(如中文、emoji 等)切开:
decoder.decode(headPart, { stream: true })会将不完整的尾部字节缓存到 decoder 内部状态- 随后
decoder.decode(tailPart, { stream: true })会将缓存字节与 tailPart 合并,导致该字符出现在 tail 而非 head
结果:不会丢失数据(TextDecoder 的 stream 模式保证了跨调用的正确拼接),但 headBufferedBytes 的实际文本内容可能比记录的字节数少几个字节。对统计/结算场景影响极小(head 缓冲区最多少几个字节),但值得了解这一行为。
🤖 Prompt for AI Agents
In `@src/app/v1/_lib/proxy/response-handler.ts` around lines 1156 - 1174, The
current byte-wise split using value.subarray(0, remainingHeadBytes) can cut a
multibyte UTF‑8 character; change the split to a safe UTF‑8 boundary: before
slicing, scan backward from remainingHeadBytes up to 3 bytes to find a byte
where (byte & 0xC0) != 0x80 (a UTF‑8 start byte) and use that index as
splitIndex; if none found, set splitIndex = remainingHeadBytes (fallback).
Replace uses of value.subarray(0, remainingHeadBytes) and
value.subarray(remainingHeadBytes) with value.subarray(0, splitIndex) and
value.subarray(splitIndex) and keep decoder.decode(..., { stream: true }) and
pushChunk calls the same (adjust counts passed to pushChunk to reflect
splitIndex instead of remainingHeadBytes). Ensure the helper logic is used in
the branch that references inTailMode, headBufferedBytes, MAX_STATS_HEAD_BYTES,
decoder, pushChunk, value, and chunkSize.
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The code demonstrates solid engineering practices across all reviewed dimensions.
PR Size: XL
- Lines changed: 4,858 (4,569 additions, 289 deletions)
- Files changed: 49
Note: This is a large release PR (v0.5.7). While reviewing, I verified that the changes are cohesive and focused on the release theme (billing header rectifier, rate limiting improvements, statistics optimization, and agent pool stability).
Review Coverage
-
Logic and correctness - Clean. The concurrent session limit resolution logic correctly handles edge cases (NaN, Infinity, negative values). The Lua script for atomic session tracking properly guards against invalid TTL values.
-
Security (OWASP Top 10) - Clean. No injection vulnerabilities detected. Redis Lua scripts use proper key/argument separation. No hardcoded secrets or credentials in the diff.
-
Error handling - Clean. Proper fail-open patterns in rate limiting (
return { allowed: true }on errors). Error logging is comprehensive with appropriate context. No silent failures detected. -
Type safety - Clean. TypeScript types are properly defined and used. Generic constraints are appropriate. No
anytype abuse detected in the changed code. -
Documentation accuracy - Clean. Comments match code behavior. The Lua script documentation accurately describes the return values and parameters. Function-level JSDoc is present and accurate.
-
Test coverage - Adequate. Unit tests cover the key concurrent session limit resolution logic with comprehensive edge cases (NaN, Infinity, negative values, decimal truncation).
-
Code clarity - Good. Functions are appropriately sized. Variable names are descriptive. The rate limit guard logic follows a clear priority order as documented.
Notable Observations
-
Good defensive programming: The
normalizePositiveLimitfunction correctly handles edge cases likeNaN,Infinity, and negative values. -
Proper Redis Lua script practices: The
CHECK_AND_TRACK_SESSIONLua script includes a guard against invalid TTL values (if ttl <= 0 then ttl = 300000 end), preventing potential issues. -
Clean agent pool cleanup: The change from
close()todestroy()with fire-and-forget pattern properly handles the blocking issue mentioned in the PR description. -
Comprehensive i18n: All 5 supported languages have been updated with the new billing header rectifier setting strings.
Automated review by Claude AI
| scope: "request", | ||
| hit: true, | ||
| removedCount: billingResult.removedCount, | ||
| extractedValues: billingResult.extractedValues, |
There was a problem hiding this comment.
[HIGH] [SECURITY-VULNERABILITY] Persisting extractedValues stores unbounded, user-controlled prompt content
Why this is a problem: billingResult.extractedValues comes from message.system (request body) and can be arbitrarily long. Persisting it into specialSettings writes raw user-controlled text to Redis/DB via persistSpecialSettings(), bypassing the default prompt redaction behavior (STORE_SESSION_MESSAGES false redacts system). This creates an avoidable data-retention/privacy risk and a potential DoS vector via oversized specialSettings payloads.
Suggested fix:
// src/app/v1/_lib/proxy/forwarder.ts
session.addSpecialSetting({
type: "billing_header_rectifier",
scope: "request",
hit: true,
removedCount: billingResult.removedCount,
// Do not persist user-controlled header text.
});
// src/types/special-settings.ts
export type BillingHeaderRectifierSpecialSetting = {
type: "billing_header_rectifier";
scope: "request";
hit: boolean;
removedCount: number;
};| if (typeof system === "string") { | ||
| if (BILLING_HEADER_PATTERN.test(system)) { | ||
| const extractedValues = [system.trim()]; | ||
| delete message.system; |
There was a problem hiding this comment.
[MEDIUM] [LOGIC-BUG] String system handling deletes the entire system prompt
Why this is a problem: In the string system case, BILLING_HEADER_PATTERN.test(system) matches when the prompt starts with x-anthropic-billing-header:. If the string prompt is multi-line (header line + real system instructions), the current code does delete message.system; and drops the remaining instructions.
Suggested fix (remove only leading billing-header lines; keep the rest):
if (typeof system === "string") {
const lines = system.split(/\r?\n/);
const extractedValues: string[] = [];
const remaining: string[] = [];
for (const line of lines) {
if (remaining.length === 0 && BILLING_HEADER_PATTERN.test(line)) {
extractedValues.push(line.trim());
continue;
}
remaining.push(line);
}
if (extractedValues.length === 0) {
return { applied: false, removedCount: 0, extractedValues: [] };
}
const nextSystem = remaining.join("\n").trim();
if (nextSystem) {
message.system = nextSystem;
} else {
delete message.system;
}
return { applied: true, removedCount: extractedValues.length, extractedValues };
}There was a problem hiding this comment.
Code Review Summary
This PR is XL and spans proxy request/response handling, rate limiting/session tracking, database schema, and UI settings, which increases rollback and review risk. The main problems are (1) persisting unbounded user-controlled billing-header text into audit storage, and (2) a request-mutation edge case that can drop legitimate system prompts in the new billing-header rectifier.
PR Size: XL
- Lines changed: 4858
- Files changed: 49
- Split suggestions:
- Billing Header Rectifier (new rectifier + system setting + i18n + tests)
- Concurrent session limit work (Redis key hashing + Lua scripts + RateLimit/Session guards + tests)
- Quota dashboard batching (repository batch SUM queries + page changes)
- Misc scripts/cleanup fixes
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 1 | 0 |
| Security | 0 | 1 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
src/app/v1/_lib/proxy/forwarder.ts:1783[HIGH] [SECURITY-VULNERABILITY] PersistingextractedValuesstores unbounded, user-controlled prompt content (bypasses redaction). (Confidence: 90/100)src/app/v1/_lib/proxy/billing-header-rectifier.ts:39[MEDIUM] [LOGIC-BUG] Stringsystemhandling deletes the entire system prompt when the billing header is prefixed. (Confidence: 85/100)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Applied PR size label size/XL, then reviewed PR #785 and submitted:
- 2 inline review comments on diff lines:
src/app/v1/_lib/proxy/forwarder.ts:1783— persistingextractedValuesstores unbounded user-controlled content (security/DoS + redaction bypass).src/app/v1/_lib/proxy/billing-header-rectifier.ts:39— stringsystempath deletes the entire system prompt (logic bug).
- A summary review comment (with XL split suggestions and issue table) via
gh pr review --comment.
Summary
Release v0.5.7 - This release introduces a new billing header rectifier feature, strengthens rate limiting and session management, improves statistics query performance, and enhances agent pool stability.
Key Changes
1. Billing Header Rectifier (New Feature)
2. Rate Limit & Session Management Improvements
3. Statistics & Query Performance
4. Agent Pool & Connection Management
5. Infrastructure & Refactoring
Database Migrations
Testing
Configuration
New system setting:
Related Issues
Release v0.5.7 - ding113