Conversation
…ers in toActionResult (#1235) getProviderGroupsWithCount and getModelSuggestionsByProviderGroup returned raw HTTP bodies, but dashboard consumers check res.ok, so the user-edit panel showed 获取供应商分组统计失败: undefined on every open. Wrap both in toActionResult; add real-wrapper contract tests and align stale model-suggestion mocks.
… focused (#1237) Fixes the create-user dialog where the "provider group" dropdown would not open to select existing groups. TagInput now auto-opens its suggestions the first time they load asynchronously while the input is focused (fire-once, disabled-guarded), so focusing the field before the data arrives no longer leaves the dropdown stuck closed. The data-fetch half of #1212 was fixed separately in #1235; this completes the interaction-layer fix. Includes regression tests (incl. the dialog/portal path) and corrects the test mock path. Fixes #1212 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tream disconnect (#1239) Narrow process-level uncaughtException/unhandledRejection suppression to EPIPE (write-side, unambiguous downstream disconnect) only; ECONNRESET/ERR_STREAM_PREMATURE_CLOSE stay fatal to preserve fail-fast. Adds getBenignBrokenPipeCode for accurate nested-code logging. Closes #1234
…1240) * fix(notification): prevent stale repeatable jobs from firing after switch off and fix cost alert interval collapse When the master switch or sub-switch is turned off, previously enqueued repeatable jobs continued to fire because the processor did not re-check settings at execution time. Additionally, the scheduleNotifications function was guarded by NODE_ENV="production", so changes in development were silently ignored. The cost alert interval >=60 minutes collapsed to an hourly cron because Bull's cron only supports minutes 0-59. - Remove NODE_ENV guard so scheduleNotifications always runs on settings save. - Add enabled/sub-switch checks in daily-leaderboard and cost-alert job processors to skip sending when disabled. - Extract removeAllRepeatableJobs and make individual key removal resilient to transient Redis errors. - For cost alert intervals >=60 minutes, use {every} instead of */N cron to schedule every N minutes correctly. - Add unit tests covering processor skip behavior, removal resilience, and cost alert interval mapping. Fixes #1236 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(notification-queue): prevent double-firing and stale alerts (#1236) The scheduler previously added new repeatable jobs even when old ones could not be removed, causing duplicate notifications. Circuit-breaker jobs also lacked a run-time switch check, so alerts could fire after the feature was disabled. - Extract clampIntervalMinutes and intervalToRepeat helpers; use cron only when the interval divides 60 evenly, falling back to the every option otherwise. This avoids uneven cron patterns and simplifies scheduling across legacy and targets modes. - Return a success boolean from removeAllRepeatableJobs and abort the reschedule if any removal fails, preventing old and new jobs from running simultaneously. - Add an execution-time guard in the circuit-breaker queue processor that re-verifies the master and circuit-breaker switches before sending, skipping with success: true, skipped: true when off. - Add unit tests for the circuit-breaker switch logic, the cost-alert positive path, targets-mode scheduling with binding jobId and timezone, and the abort-on-removal-failure scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(notification): add circuit-breaker sub-switch-off skip test Add a test case verifying that the notification queue processor skips sending when circuitBreakerEnabled is false. Covers a scenario flagged as missing during code review. Refs #1236 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nc overwrite (#1241) 模型价格表「本地优先」修复: - 用户显式上传的价格表(uploadPriceTable)标记为 source='manual',不再被云端自动同步静默覆盖。 - 跳过保护仅在 source='litellm'(云端/自动同步)时生效;手动上传为权威导入,可正常覆盖与重新上传更新。 - update 路径改用事务型 upsertModelPrice 原子替换(delete+insert 同事务),消除崩溃丢数据窗口并清理孤儿行。 - 云端模型名 trim 归一化后再做保护比对。 - 新增针对上传/重新上传/转换/归一化的单元测试。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough此 PR 升级 Node 到 22、在代理层实现 Content-Encoding 解码和请求头剥离、将 model-prices 引入 source 并用 upsert 实现本地优先、重构通知调度与队列、添加良性 EPIPE 抑制、扩展 usage-logs 为 CSV/XLSX（含 XLSX 生成与异步任务），并补充大量测试与 UI/i18n 调整。 Changes全部更改（单一审阅切片）
Estimated code review effort: Possibly related PRs:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
github-actions
Bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces native request body decompression (zstd, gzip, deflate, br) in the proxy layer, safe handling of benign client disconnects (EPIPE) to prevent process crashes, a 'local-first' model pricing sync strategy, and improved notification scheduling. The code reviewer provided several valuable recommendations: lowering the 100MB decompression limit to mitigate DoS risks, deleting the content-length header alongside content-encoding to prevent upstream truncation, defensively handling NaN values in clampIntervalMinutes, adopting a more granular fallback when notification job removals fail, and wrapping model price updates in a single database transaction to avoid N+1 performance bottlenecks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| * next.config.ts 的 proxyClientMaxBodySize 钳制（见 proxy.matcher.ts），故入站压缩体 | ||
| * 体积本身不另设限。逐层解压按此上限增量限制，超过即按 413 拒绝。 | ||
| */ | ||
| export const MAX_DECOMPRESSED_REQUEST_BYTES = 100 * 1024 * 1024; |
There was a problem hiding this comment.
A default limit of 100MB for decompressed request bodies is extremely high for typical LLM chat/completion APIs (where even a 2-million token prompt is usually under 10MB). Since decompression, TextDecoder.decode(), and JSON.parse() are all executed synchronously on the main thread, a 100MB payload can block the Node.js event loop for several seconds and consume hundreds of megabytes of memory, making the server highly vulnerable to Denial of Service (DoS) attacks (e.g., decompression bombs). Consider lowering this default limit significantly (e.g., to 10MB or 20MB) or making it configurable via environment variables.
| export const MAX_DECOMPRESSED_REQUEST_BYTES = 100 * 1024 * 1024; | |
| export const MAX_DECOMPRESSED_REQUEST_BYTES = Number(process.env.MAX_DECOMPRESSED_REQUEST_BYTES) || 10 * 1024 * 1024; |
| if (bodyResult.decodedContentEncoding) { | ||
| headers.delete("content-encoding"); | ||
| } |
There was a problem hiding this comment.
When a request body is decompressed by the proxy, the content-encoding header is correctly deleted. However, the original content-length header (which corresponds to the compressed body size) is still preserved in the headers object. If the outbound forwarding logic does not explicitly strip or recalculate content-length, the upstream provider will receive the decompressed (much larger) body with a tiny content-length header, leading to request truncation or protocol errors. To prevent this, explicitly delete the content-length header as well so that the HTTP client can automatically recalculate it based on the decompressed payload.
| if (bodyResult.decodedContentEncoding) { | |
| headers.delete("content-encoding"); | |
| } | |
| if (bodyResult.decodedContentEncoding) { | |
| headers.delete("content-encoding"); | |
| headers.delete("content-length"); | |
| } |
| function clampIntervalMinutes(rawMinutes: number): number { | ||
| return Math.min(Math.max(1, Math.trunc(rawMinutes)), 24 * 60); | ||
| } |
There was a problem hiding this comment.
If rawMinutes is NaN, Math.trunc(rawMinutes) will return NaN. This NaN value will propagate through Math.max and Math.min, causing clampIntervalMinutes to return NaN. Consequently, intervalToRepeat will return { every: NaN }, which will cause Bull/Redis to throw a runtime error when scheduling the job. Ensure that NaN or non-finite values are handled defensively by falling back to a default interval (e.g., 60 minutes).
| function clampIntervalMinutes(rawMinutes: number): number { | |
| return Math.min(Math.max(1, Math.trunc(rawMinutes)), 24 * 60); | |
| } | |
| function clampIntervalMinutes(rawMinutes: number): number { | |
| const minutes = Number.isFinite(rawMinutes) ? Math.trunc(rawMinutes) : 60; | |
| return Math.min(Math.max(1, minutes), 24 * 60); | |
| } |
References
- When scheduling repeatable tasks using Bull queue, only use a cron expression if the interval in minutes is less than or equal to 59 and divides 60 evenly. For intervals that do not divide 60 or are 60 or greater, use a fixed millisecond interval to prevent uneven execution intervals at hour boundaries.
| const removedAll = await removeAllRepeatableJobs(queue); | ||
| if (!removedAll) { | ||
| logger.error({ | ||
| action: "schedule_notifications_aborted", | ||
| reason: "stale_repeatable_remove_failed", | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Aborting the entire notification scheduling if a single repeatable job fails to be removed might be too aggressive and could lead to a complete outage of all notifications (e.g., cost alerts, daily leaderboards, cache hit rate alerts) due to a transient Redis issue with a single job key. Consider a more granular approach where only the failed notification types are skipped, or proceed with a warning, as duplicate notifications are often preferable to a total notification blackout.
|
|
||
| // 处理每个模型的价格 | ||
| for (const [modelName, priceData] of entries) { | ||
| for (const [rawModelName, priceData] of entries) { |
There was a problem hiding this comment.
Performing individual database transactions (await createModelPrice / await upsertModelPrice) inside a loop for each model entry can lead to severe performance bottlenecks (N+1 transactions) when processing large price tables. Consider wrapping the entire loop inside a single database transaction (db.transaction) to commit all changes atomically and significantly reduce database I/O overhead.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -2,6 +2,9 @@ | |||
| "name": "claude-code-hub", | |||
| "version": "0.8.0", | |||
| "private": true, | |||
There was a problem hiding this comment.
The PR title is "release v0.8.4" but package.json still declares "version": "0.8.0". Any build artifact, API response, or UI element that reads the package version will report 0.8.0, making it impossible to distinguish this release from prior ones by version string alone.
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 4
Comment:
**Version not bumped for this release**
The PR title is "release v0.8.4" but `package.json` still declares `"version": "0.8.0"`. Any build artifact, API response, or UI element that reads the package version will report 0.8.0, making it impossible to distinguish this release from prior ones by version string alone.
How can I resolve this? If you propose a fix, please make it concise.| RUN --mount=type=cache,target=/app/.next/cache bun run build | ||
|
|
||
| FROM node:20-slim AS runner | ||
| FROM node:22-slim AS runner |
There was a problem hiding this comment.
node:22-slim is a rolling tag that can resolve to any 22.x patch release. The zstdDecompressSync API (used in request-body-codec.ts) was first shipped in Node 22.15.0. If this image is built against a stale Docker cache that points to an earlier 22.x build, the decompression call will fail at runtime with TypeError: zstdDecompressSync is not a function. Pinning to node:22.15-slim (or later) makes the minimum requirement explicit and matches what package.json engines already declares.
| FROM node:22-slim AS runner | |
| FROM node:22.15-slim AS runner |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Dockerfile
Line: 15
Comment:
`node:22-slim` is a rolling tag that can resolve to any 22.x patch release. The `zstdDecompressSync` API (used in `request-body-codec.ts`) was first shipped in Node 22.15.0. If this image is built against a stale Docker cache that points to an earlier 22.x build, the decompression call will fail at runtime with `TypeError: zstdDecompressSync is not a function`. Pinning to `node:22.15-slim` (or later) makes the minimum requirement explicit and matches what `package.json` `engines` already declares.
```suggestion
FROM node:22.15-slim AS runner
```
How can I resolve this? If you propose a fix, please make it concise.| if (encodings.length > MAX_CONTENT_ENCODING_LAYERS) { | ||
| // 防御多层编码放大：每层都是一次同步解压，过多层数纯属攻击/异常。 | ||
| throw new ProxyError( | ||
| `Too many content-encoding layers (${encodings.length}); at most ${MAX_CONTENT_ENCODING_LAYERS} are allowed.`, | ||
| 400 | ||
| ); | ||
| } | ||
|
|
||
| const unsupported = encodings.filter((enc) => !SUPPORTED_ENCODINGS.has(enc)); |
There was a problem hiding this comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/request-body-codec.ts
Line: 163-171
Comment:
The multi-layer encoding check happens **after** the unsupported-encoding passthrough, so a request with two layers of supported encodings correctly gets a 400, while one with an unsupported outer encoding is passed through without hitting the layer limit. The ordering is intentional, but the inline comment doesn't mention that the passthrough also bypasses the layer-count guard — a future reader tightening the limit might not realise this. Consider adding a note to make the intentional bypass explicit.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/model-prices.ts (1)
142-142:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win统一覆盖列表与导入键的归一化，避免同轮重复写入同一模型。
Line 142 的
overwriteSet未做trim()，且 Line 164-167 才对模型名归一化；当输入同时出现"model"与" model "时，会在同一轮被当作两条记录处理，导致覆盖判断偏差和重复写入风险。建议修改
- const entries = Object.entries(priceTable).filter(([modelName]) => { - // 排除元数据字段 - if (METADATA_FIELDS.includes(modelName)) { - logger.debug(`跳过元数据字段: ${modelName}`); - return false; - } - return typeof modelName === "string" && modelName.trim().length > 0; - }); - - // 创建覆盖列表的 Set 用于快速查找 - const overwriteSet = new Set(overwriteManual ?? []); + const normalizedEntries = new Map<string, unknown>(); + for (const [rawModelName, priceData] of Object.entries(priceTable)) { + if (METADATA_FIELDS.includes(rawModelName)) { + logger.debug(`跳过元数据字段: ${rawModelName}`); + continue; + } + const modelName = rawModelName.trim(); + if (!modelName) continue; + // 同名归一化后保留最后一次定义，避免同轮重复写入 + normalizedEntries.set(modelName, priceData); + } + const entries = Array.from(normalizedEntries.entries()); + + // 创建覆盖列表的 Set 用于快速查找（与 modelName 归一化保持一致） + const overwriteSet = new Set((overwriteManual ?? []).map((name) => name.trim()).filter(Boolean)); @@ - for (const [rawModelName, priceData] of entries) { - // 与 manual 记录入库时（upsertModelPrice 使用 trim 后的名称）保持一致地归一化， - // 避免云端表里带空白的同名键绕过本地手动模型的保护检查。 - const modelName = typeof rawModelName === "string" ? rawModelName.trim() : rawModelName; + for (const [modelName, priceData] of entries) {Also applies to: 164-167, 187-187
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/actions/model-prices.ts` at line 142, Normalize entries when creating overwriteSet so keys match the later model-name normalization: trim whitespace (and apply the same normalization rules used in the block around the existing model-name normalization at lines 164-167) for every value from overwriteManual before adding to the Set; also apply the same normalized trimming behavior for any other user-provided model key uses (e.g., the logic referenced near line 187) so "model" and " model " are treated identically by overwriteSet and the overwrite-check logic.
🧹 Nitpick comments (2)
src/app/v1/_lib/proxy/request-body-codec.ts (1)
25-25: ⚡ Quick win导入路径建议改为
@/别名。Line 25 新增了相对导入，和仓库 TS 导入规范不一致。建议改为
@/app/v1/_lib/proxy/errors形式。As per coding guidelines "Use path alias
@/to map to ./src/ for imports".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/request-body-codec.ts` at line 25, Update the import for ProxyError in request-body-codec.ts to use the project path-alias instead of a relative path: replace the relative import that references "./errors" with the aliased module "`@/app/v1/_lib/proxy/errors`" so the ProxyError symbol is imported via the TS `@/` -> ./src/ mapping and adheres to the repository import convention.src/app/v1/_lib/proxy/session.ts (1)
32-32: ⚡ Quick win新增导入建议使用
@/别名路径。Line 32 建议改为
@/app/v1/_lib/proxy/request-body-codec，保持与仓库导入规范一致。As per coding guidelines "Use path alias
@/to map to ./src/ for imports".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/session.ts` at line 32, Import uses a relative path for decodeRequestBody; change the import in src/app/v1/_lib/proxy/session.ts to use the repository alias by replacing the relative module path with "`@/app/v1/_lib/proxy/request-body-codec`" so the symbol decodeRequestBody is imported via the project alias consistent with the coding guideline "Use path alias `@/` to map to ./src/".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-check.yml:
- Line 34: 将 CI 的 Node 版本从宽泛的 "22" 锁定到具体下限以避免运行时漂移：在
`.github/workflows/pr-check.yml` 中把 node-version 字段的值改为
"22.15.x"（或更高的明确版本），确保使用的 Node API（如 zstdDecompressSync）在 CI 环境中可用并且不会因 22.x
的不同次版本而失败；更新后运行一次 CI 以验证兼容性。
In `@Dockerfile`:
- Line 15: The Dockerfile uses a floating runtime image tag ("FROM
node:22-slim"); change this to a fully pinned patch-level image (for example
"node:22.15.0-slim" or a specific distro variant like
"node:22.15.0-bookworm-slim") to ensure reproducible builds—update the FROM
instruction that currently references node:22-slim to the chosen exact patch tag
across build stages.
In `@src/actions/notifications.ts`:
- Around line 43-46: The scheduleNotifications import/call can throw and
currently bubbles to the outer catch after updateNotificationSettings(payload)
succeeded; wrap the dynamic import and await scheduleNotifications() in their
own try/catch so any errors are caught, logged (or processLogger.warn/error) and
not rethrown—i.e., perform a fail-open: use try { const { scheduleNotifications
} = await import("`@/lib/notification/notification-queue`"); await
scheduleNotifications(); } catch (err) { /* log err but do not throw */ } to
ensure updateNotificationSettings completes successfully even if scheduling
fails.
---
Outside diff comments:
In `@src/actions/model-prices.ts`:
- Line 142: Normalize entries when creating overwriteSet so keys match the later
model-name normalization: trim whitespace (and apply the same normalization
rules used in the block around the existing model-name normalization at lines
164-167) for every value from overwriteManual before adding to the Set; also
apply the same normalized trimming behavior for any other user-provided model
key uses (e.g., the logic referenced near line 187) so "model" and " model " are
treated identically by overwriteSet and the overwrite-check logic.
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/request-body-codec.ts`:
- Line 25: Update the import for ProxyError in request-body-codec.ts to use the
project path-alias instead of a relative path: replace the relative import that
references "./errors" with the aliased module "`@/app/v1/_lib/proxy/errors`" so
the ProxyError symbol is imported via the TS `@/` -> ./src/ mapping and adheres to
the repository import convention.
In `@src/app/v1/_lib/proxy/session.ts`:
- Line 32: Import uses a relative path for decodeRequestBody; change the import
in src/app/v1/_lib/proxy/session.ts to use the repository alias by replacing the
relative module path with "`@/app/v1/_lib/proxy/request-body-codec`" so the symbol
decodeRequestBody is imported via the project alias consistent with the coding
guideline "Use path alias `@/` to map to ./src/".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ba0c110-2a7c-4958-90d6-a568df2f0e1c
📒 Files selected for processing (28)
.github/workflows/pr-check.ymlDockerfileREADME.en.mdREADME.mddeploy/Dockerfilepackage.jsonsrc/actions/model-prices.tssrc/actions/notifications.tssrc/app/v1/_lib/models/available-models.tssrc/app/v1/_lib/proxy/request-body-codec.tssrc/app/v1/_lib/proxy/session.tssrc/components/ui/__tests__/provider-group-tag-input.test.tsxsrc/components/ui/tag-input.tsxsrc/instrumentation.tssrc/lib/api-client/v1/actions/providers.tssrc/lib/lifecycle/benign-errors.tssrc/lib/notification/notification-queue.tssrc/repository/model-price.tstests/unit/actions/model-prices.test.tstests/unit/api/v1/api-client-actions.test.tstests/unit/benign-broken-pipe-error.test.tstests/unit/instrumentation-crash-handler.test.tstests/unit/notification/notification-queue.test.tstests/unit/proxy/available-models.test.tstests/unit/proxy/request-body-codec.test.tstests/unit/proxy/session-request-decode.test.tstests/unit/settings/providers/provider-form-endpoint-pool.test.tsxtests/unit/settings/providers/provider-form-total-limit-ui.test.tsx
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: '20' | ||
| node-version: '22' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
在 Node.js 官方文档中确认：node:zlib的 zstd 同步解压（如zstdDecompressSync）从哪个 Node 版本开始可用？Node 22.0.0 是否保证可用？
💡 Result:
在 Node.js 中，node:zlib 模块提供的 Zstd 同步解压方法 zstdDecompressSync 是从 Node.js v22.15.0 版本开始可用的 [1][2]。 关于您询问的 Node 22.0.0 版本：该功能在 Node 22.0.0 中不可用 [2]。zstdDecompressSync 直到后续的补丁版本 v22.15.0 才被正式加入到 API 中 [1][2]。如果您需要在 Node 22.x 系列中使用此功能，请确保您的 Node.js 版本至少升级到 v22.15.0 或更高版本。
Citations:
- 1: https://nodejs.org/docs/latest-v24.x/api/zlib.html
- 2: https://nodejs.org/docs/latest-v22.x/api/zlib.html
CI Node 版本应固定到明确下限（22.15.x）
当前 .github/workflows/pr-check.yml 将 node-version 固定为 "22"，可能解析到 22.15.0 之前的 22.x；而 node:zlib 的 zstdDecompressSync 需要至少 Node.js v22.15.0 才提供（22.0.0 不包含该能力）。为避免 CI 运行时漂移，建议固定到 22.15.x（或更高）。
建议修改
- node-version: '22'
+ node-version: "22.15.x"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-check.yml at line 34, 将 CI 的 Node 版本从宽泛的 "22"
锁定到具体下限以避免运行时漂移：在 `.github/workflows/pr-check.yml` 中把 node-version 字段的值改为
"22.15.x"（或更高的明确版本），确保使用的 Node API（如 zstdDecompressSync）在 CI 环境中可用并且不会因 22.x
的不同次版本而失败；更新后运行一次 CI 以验证兼容性。
| RUN --mount=type=cache,target=/app/.next/cache bun run build | ||
|
|
||
| FROM node:20-slim AS runner | ||
| FROM node:22-slim AS runner |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node 官方 Docker 镜像是否提供 node:22.15-slim（或 22.15.x-slim）标签？推荐的生产固定标签策略是什么？
💡 Result:
是的，Node 官方 Docker 镜像提供 node:22.15-slim 这一类“具体版本+slim”的标签；但从官方文档当前维护的标签列表来看，更常见/推荐使用的精确固定形式是 node:22.15.0-slim（或 node:22.15.0-*），因为 22.15.x-slim/22.15-slim 这类“模糊 x”并不是官方文档中最标准的命名方式。 证据：Docker Hub 对 node:22.15-slim 有对应镜像条目（可拉取的标签存在）[1]，同时官方镜像文档列出了 node:<version>-slim 体系及具体支持标签形如 22.15-slim、22.15.0-slim（以及不同 Debian 发行版变体）[2]。 推荐的生产固定标签策略： 1) 固定到“完整版本号 + 变体” - 直接使用类似 node:22.15.0-slim（或你需要的 -bullseye-slim/-bookworm-slim/-trixie-slim 等后缀），避免使用 node:22-slim、node:lts-slim、node:current-slim 这类会随时间移动的浮动标签。[3][2] 2) 记录并更新策略（自动更新但可回滚） - 用固定精确标签保证可复现；当需要升级时再显式把镜像版本从旧的 22.15.0-slim 变为新版本（例如下一次补丁/次版本），由 CI/CD 触发验证后再发布。 3) 如需“最小体积”，才用 -slim - 官方对 -slim 的描述是：它不包含默认标签里的常见包，只包含运行 node 所需的最少组件；除非你明确在空间敏感场景，否则建议优先使用默认镜像（但这不改变“生产固定到精确版本”的策略）[4][2]。 如果你告诉我你希望基于哪个 Debian 发行版（bookworm/bullseye/trixie）以及是否需要 openssl/构建工具等，我也可以给出你可以直接写进 Dockerfile/compose 的具体推荐标签组合（如 node:22.15.0-bookworm-slim vs node:22.15.0-slim）。
Citations:
- 1: https://hub.docker.com/layers/library/node/22.15-slim/images/sha256-ed0e340edf19b7014fd6b0a5f7048b73826b6ae6104132184243f9422b1e9957
- 2: https://github.com/docker-library/docs/blob/master/node/README.md
- 3: https://hub.docker.com/_/node/?tab=tags
- 4: https://hub.docker.com/_/node?tab=tags
建议将运行时基础镜像固定到精确补丁版本，避免使用漂移标签（Dockerfile 第15行：node:22-slim）
node:22-slim/类似“宽泛标签”会随时间移动，影响可重复性；而engines >=22.15.0更适合用精确匹配的node:22.15.0-slim（或按需选择-bookworm-slim/-bullseye-slim等发行版变体）来固定运行时。- 目前虽然存在
node:22.15-slim，但生产实践上仍更建议直接 pin 到完整版本（到.0）以减少补丁层面的漂移风险。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` at line 15, The Dockerfile uses a floating runtime image tag
("FROM node:22-slim"); change this to a fully pinned patch-level image (for
example "node:22.15.0-slim" or a specific distro variant like
"node:22.15.0-bookworm-slim") to ensure reproducible builds—update the FROM
instruction that currently references node:22-slim to the chosen exact patch tag
across build stages.
| // 重新调度通知任务，使总开关、子开关、时间/间隔等变更立即生效（添加/移除 repeatable 作业）。 | ||
| // 动态导入避免静态加载 Bull；scheduleNotifications 内部已 fail-open，缺少 REDIS_URL 时不会影响设置保存。 | ||
| const { scheduleNotifications } = await import("@/lib/notification/notification-queue"); | ||
| await scheduleNotifications(); |
There was a problem hiding this comment.
将调度调用改为真正的 fail-open，避免“设置已保存但返回失败”
这里如果 import() 或 scheduleNotifications() 抛错，会进入外层 catch，但 updateNotificationSettings(payload) 已经成功提交，接口会返回失败并可能诱发重复提交。
建议修改
- const { scheduleNotifications } = await import("`@/lib/notification/notification-queue`");
- await scheduleNotifications();
+ try {
+ const { scheduleNotifications } = await import("`@/lib/notification/notification-queue`");
+ await scheduleNotifications();
+ } catch {
+ // fail-open: 调度失败不影响设置保存结果
+ }📝 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.
| // 重新调度通知任务，使总开关、子开关、时间/间隔等变更立即生效（添加/移除 repeatable 作业）。 | |
| // 动态导入避免静态加载 Bull；scheduleNotifications 内部已 fail-open，缺少 REDIS_URL 时不会影响设置保存。 | |
| const { scheduleNotifications } = await import("@/lib/notification/notification-queue"); | |
| await scheduleNotifications(); | |
| // 重新调度通知任务，使总开关、子开关、时间/间隔等变更立即生效（添加/移除 repeatable 作业）。 | |
| // 动态导入避免静态加载 Bull；scheduleNotifications 内部已 fail-open，缺少 REDIS_URL 时不会影响设置保存。 | |
| try { | |
| const { scheduleNotifications } = await import("`@/lib/notification/notification-queue`"); | |
| await scheduleNotifications(); | |
| } catch { | |
| // fail-open: 调度失败不影响设置保存结果 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/actions/notifications.ts` around lines 43 - 46, The scheduleNotifications
import/call can throw and currently bubbles to the outer catch after
updateNotificationSettings(payload) succeeded; wrap the dynamic import and await
scheduleNotifications() in their own try/catch so any errors are caught, logged
(or processLogger.warn/error) and not rethrown—i.e., perform a fail-open: use
try { const { scheduleNotifications } = await
import("`@/lib/notification/notification-queue`"); await scheduleNotifications();
} catch (err) { /* log err but do not throw */ } to ensure
updateNotificationSettings completes successfully even if scheduling fails.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| RUN --mount=type=cache,target=/app/.next/cache bun run build | ||
|
|
||
| FROM node:20-slim AS runner | ||
| FROM node:22-slim AS runner |
There was a problem hiding this comment.
[Medium] [STANDARD-VIOLATION] Rolling Docker tag doesn't guarantee the minimum Node version required by engines
package.json declares "engines": { "node": ">=22.15.0" } and request-body-codec.ts relies on zstdDecompressSync (available since 22.15). The node:22-slim tag is rolling and can resolve to any 22.x patch. If Docker build cache is stale and resolves to a pre-22.15 image, zstdDecompressSync will throw TypeError at runtime for any zstd-encoded request body.
The deploy/Dockerfile already uses node:trixie-slim (Node 24+) with a comment explaining the 22.15 requirement. The main Dockerfile should be similarly explicit.
Suggested fix:
FROM node:22.15-slim AS runner
There was a problem hiding this comment.
Code Review Summary
This release PR (v0.8.4) introduces compressed request body support (zstd/gzip/deflate/br), benign EPIPE suppression, notification scheduling fixes, local-first model price protection, and UI regression fixes. The new code is well-structured with strong test coverage (~1,400 lines of new tests). Two issues were identified.
PR Size: L
- Lines changed: 2,172 (2,049 additions + 123 deletions)
- Files changed: 28
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 1 | 0 |
High Priority Issues (Should Fix)
1. [LOGIC-BUG] Missing runtime switch guard for cache-hit-rate-alert notification type
src/lib/notification/notification-queue.ts ~line 495 (dev branch)
This PR adds runtime switch guards to the circuit-breaker, daily-leaderboard, and cost-alert processor cases, but omits the cache-hit-rate-alert case. The PR's stated goal is "Fix notification scheduling -- master/sub switches still fire when master switch is off". Without a guard, an existing repeatable cache-hit-rate-alert job will still execute and send notifications even after the user disables the master switch or cacheHitRateAlertEnabled sub-switch.
All three other cases follow the same guard pattern:
const { getNotificationSettings } = await import("@/repository/notifications");
const settings = await getNotificationSettings();
if (\!settings.enabled || \!settings.<subSwitch>Enabled) {
logger.info({ action: "<type>_disabled", jobId: job.id });
return { success: true, skipped: true };
}Suggested fix — add the same guard to the cache-hit-rate-alert case, before the payload generation logic:
case "cache-hit-rate-alert": {
const { getNotificationSettings } = await import("@/repository/notifications");
const settings = await getNotificationSettings();
if (\!settings.enabled || \!settings.cacheHitRateAlertEnabled) {
logger.info({ action: "cache_hit_rate_alert_disabled", jobId: job.id });
return { success: true, skipped: true };
}
// ... existing payload generation logicMedium Priority Issues
2. [STANDARD-VIOLATION] Dockerfile rolling tag doesn't guarantee minimum Node 22.15 (inline comment posted)
node:22-slim is a rolling tag; package.json engines requires >=22.15.0 for native zstdDecompressSync. Stale Docker cache could produce an image where zstd decompression fails silently. Pin to node:22.15-slim to match the engines declaration (deploy/Dockerfile already uses node:trixie-slim for Node 24+).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Claude AI
请求过滤器与错误规则在增删改/启停后立即同步代理内存缓存并刷新 UI 列表，无需手动点刷新缓存。RequestFilterEngine.reload() 增加并发补跑队列；mutation action 写库后 await reload（best-effort，失败不影响写入成功语义，并复用 emit 触发的在途 reload 避免双读）；error-rules UI 各操作补 router.refresh()。处理了 Gemini/CodeRabbit 的 review 意见。
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d549ca54
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // 非 multipart：按 content-encoding（zstd/gzip/deflate/br）解压请求体， | ||
| // 使下游模型解析、过滤、计费、日志与转发都基于明文。 | ||
| const decodedBody = decodeRequestBody(rawBodyBuffer, contentEncoding); | ||
| const requestBodyBuffer = decodedBody.buffer; |
There was a problem hiding this comment.
For requests with supported content-encoding, this synchronously decompresses the body while ProxySession.fromContext is still building the session, before the auth guard has rejected invalid tokens. Because the proxy path has no compressed input-size cap and allows up to 100 MB of decompressed output, an unauthenticated client can repeatedly send small gzip/zstd bombs to force large CPU and memory work before authorization; move the auth decision ahead of decompression or add a much tighter pre-auth compressed/decompressed limit.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
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/app/[locale]/settings/error-rules/_components/rule-list-table.tsx (1)
157-178:⚠️ Potential issue | 🟠 Major | ⚡ Quick win为这些仅图标控件补上可访问名称。
这里的
Switch、编辑按钮和删除按钮都没有可访问名称，读屏器只会读成“switch / button”，用户无法判断具体操作，会直接影响错误规则管理流程。建议为每个控件补aria-label（必要时再加title），并通过t(...)提供翻译；这样测试也能改成按 role/name 查询，不再依赖图标类名。As per coding guidelines "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/settings/error-rules/_components/rule-list-table.tsx around lines 157 - 178, Add accessible, translatable names for the icon-only controls: supply aria-label (and title if desired) to the Switch and both Buttons using the i18n function t(...). For the Switch in the rule row (component using Switch and handleToggleEnabled) provide a label that reflects the rule state or action (e.g., t('rules.enable_rule', { name: rule.pattern }) or t('rules.disable_rule', ... ) depending on rule.isEnabled). For the edit Button (handleEdit + Pencil) add aria-label/title like t('rules.edit_rule', { name: rule.pattern }). For the delete Button (handleDelete + Trash2) add aria-label/title like t('rules.delete_rule', { name: rule.pattern }) and preserve disabled behavior when rule.isDefault. Ensure all strings are added to the i18n resource files for the supported locales.
🧹 Nitpick comments (3)
tests/unit/actions/error-rules-cache-reload.test.ts (1)
68-135: ⚡ Quick win补一条
refreshCacheAction()的参数断言测试。这个套件现在只覆盖了 create/update/delete，但本次改动里
refreshCacheAction()的关键语义是reload({ queueIfRunning: true })。如果后续有人回退成普通reload()，当前测试不会报警。建议补测
+ it("refreshCacheAction queues a follow-up reload when one is already running", async () => { + const { refreshCacheAction } = await import("`@/actions/error-rules`"); + const res = await refreshCacheAction(); + + expect(res.ok).toBe(true); + expect(reloadMock).toHaveBeenCalledWith({ queueIfRunning: true }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/actions/error-rules-cache-reload.test.ts` around lines 68 - 135, Add a unit test for refreshCacheAction to assert it calls reload with the expected options; specifically, import refreshCacheAction from "`@/actions/error-rules`" in the test suite and verify that calling refreshCacheAction() results in reload being invoked with the object { queueIfRunning: true } (i.e., expect(reloadMock).toHaveBeenCalledWith({ queueIfRunning: true })); this ensures the key semantic in refreshCacheAction (calling reload({ queueIfRunning: true })) is covered so regressions to a plain reload() will fail the test.tests/unit/actions/request-filters-cache-reload.test.ts (1)
105-123: ⚡ Quick win把
reload(false)的参数契约也断言到 update/delete。这组测试目前只在 create 场景验证了
reload(false)，update/delete 只检查“被调用过”。如果后续有人把它们改回reload()，测试仍会通过，但会重新引入额外 DB 读取。建议和 create 一样显式断言toHaveBeenCalledWith(false)。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/actions/request-filters-cache-reload.test.ts` around lines 105 - 123, 测试中 updateRequestFilterAction 和 deleteRequestFilterAction 仅断言 reloadMock 被调用，但未验证参数；请在对应测试块中像 create 场景一样断言 reloadMock 被调用时传入 false（即为 updateRequestFilterAction 和 deleteRequestFilterAction 的断言加入 expect(reloadMock).toHaveBeenCalledWith(false) 或等价断言），以确保它们调用 reload(false) 而不是无参 reload()，参照测试中使用的 updateRequestFilterAction、deleteRequestFilterAction 和 reloadMock 标识定位并修改相应断言。tests/unit/error-rules-list-refresh-ui.test.tsx (1)
191-304: ⚡ Quick win把失败分支也补进这组回归测试。
现在只证明了“成功后会
router.refresh()”，但没有覆盖“action 返回ok: false或抛错时绝不能 refresh”这个同样关键的契约。若后续有人把router.refresh()挪到错误分支外侧，这组测试仍会全部通过。建议至少为 toggle、delete 和 refresh-cache 各补一个失败路径断言：expect(refreshMock).not.toHaveBeenCalled()。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/error-rules-list-refresh-ui.test.tsx` around lines 191 - 304, The tests only assert router.refresh() is called on successful actions; add failing-branch tests for toggle (RuleListTable + updateErrorRuleActionMock), delete (RuleListTable + deleteErrorRuleActionMock) and refresh-cache (RefreshCacheButton + refreshCacheActionMock) that simulate the action returning { ok: false } or throwing, then assert refreshMock.not.toHaveBeenCalled(); modify the corresponding mocks (updateErrorRuleActionMock, deleteErrorRuleActionMock, refreshCacheActionMock) before triggering the UI interactions and verify the UI still behaves but does not call refreshMock in the failure case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/actions/error-rules.ts`:
- Around line 179-187: The call to emitErrorRulesUpdated() should be made
best-effort so transient pub/sub/Redis failures don't cause the whole action to
return ok: false; wrap the emitErrorRulesUpdated() call in its own try/catch
(similar to how errorRuleDetector.reload() is handled) and on error log a
warning with context (e.g., "[ErrorRulesAction] Failed to emit rules update")
but do not rethrow; apply the same change to the other occurrences referenced
(the other emitErrorRulesUpdated() calls around the 321-327 and 382-388 regions)
so emits never bubble errors back to the caller.
---
Outside diff comments:
In `@src/app/`[locale]/settings/error-rules/_components/rule-list-table.tsx:
- Around line 157-178: Add accessible, translatable names for the icon-only
controls: supply aria-label (and title if desired) to the Switch and both
Buttons using the i18n function t(...). For the Switch in the rule row
(component using Switch and handleToggleEnabled) provide a label that reflects
the rule state or action (e.g., t('rules.enable_rule', { name: rule.pattern })
or t('rules.disable_rule', ... ) depending on rule.isEnabled). For the edit
Button (handleEdit + Pencil) add aria-label/title like t('rules.edit_rule', {
name: rule.pattern }). For the delete Button (handleDelete + Trash2) add
aria-label/title like t('rules.delete_rule', { name: rule.pattern }) and
preserve disabled behavior when rule.isDefault. Ensure all strings are added to
the i18n resource files for the supported locales.
---
Nitpick comments:
In `@tests/unit/actions/error-rules-cache-reload.test.ts`:
- Around line 68-135: Add a unit test for refreshCacheAction to assert it calls
reload with the expected options; specifically, import refreshCacheAction from
"`@/actions/error-rules`" in the test suite and verify that calling
refreshCacheAction() results in reload being invoked with the object {
queueIfRunning: true } (i.e., expect(reloadMock).toHaveBeenCalledWith({
queueIfRunning: true })); this ensures the key semantic in refreshCacheAction
(calling reload({ queueIfRunning: true })) is covered so regressions to a plain
reload() will fail the test.
In `@tests/unit/actions/request-filters-cache-reload.test.ts`:
- Around line 105-123: 测试中 updateRequestFilterAction 和 deleteRequestFilterAction
仅断言 reloadMock 被调用，但未验证参数；请在对应测试块中像 create 场景一样断言 reloadMock 被调用时传入 false（即为
updateRequestFilterAction 和 deleteRequestFilterAction 的断言加入
expect(reloadMock).toHaveBeenCalledWith(false) 或等价断言），以确保它们调用 reload(false)
而不是无参 reload()，参照测试中使用的 updateRequestFilterAction、deleteRequestFilterAction 和
reloadMock 标识定位并修改相应断言。
In `@tests/unit/error-rules-list-refresh-ui.test.tsx`:
- Around line 191-304: The tests only assert router.refresh() is called on
successful actions; add failing-branch tests for toggle (RuleListTable +
updateErrorRuleActionMock), delete (RuleListTable + deleteErrorRuleActionMock)
and refresh-cache (RefreshCacheButton + refreshCacheActionMock) that simulate
the action returning { ok: false } or throwing, then assert
refreshMock.not.toHaveBeenCalled(); modify the corresponding mocks
(updateErrorRuleActionMock, deleteErrorRuleActionMock, refreshCacheActionMock)
before triggering the UI interactions and verify the UI still behaves but does
not call refreshMock in the failure case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5b6e3f7-371d-41ee-8621-97c6ea7a3fd8
📒 Files selected for processing (12)
src/actions/error-rules.tssrc/actions/request-filters.tssrc/app/[locale]/settings/error-rules/_components/add-rule-dialog.tsxsrc/app/[locale]/settings/error-rules/_components/edit-rule-dialog.tsxsrc/app/[locale]/settings/error-rules/_components/refresh-cache-button.tsxsrc/app/[locale]/settings/error-rules/_components/rule-list-table.tsxsrc/lib/error-rule-detector.tssrc/lib/request-filter-engine.tstests/unit/actions/error-rules-cache-reload.test.tstests/unit/actions/request-filters-cache-reload.test.tstests/unit/error-rules-list-refresh-ui.test.tsxtests/unit/lib/request-filter-engine-reload-queue.test.ts
| // 刷新缓存（事件广播，支持多 worker 同步） | ||
| await emitErrorRulesUpdated(); | ||
| // 上面的 emit 已在本进程触发一次携带最新数据的 reload，这里复用它即可（无需补跑第二轮）。 | ||
| // reload 仅为本进程缓存同步（跨 worker 由 emit 覆盖），失败不应把已成功的写入误报为失败。 | ||
| try { | ||
| await errorRuleDetector.reload(); | ||
| } catch (reloadError) { | ||
| logger.warn("[ErrorRulesAction] Failed to reload detector after mutation", { reloadError }); | ||
| } |
There was a problem hiding this comment.
把 emitErrorRulesUpdated() 也改成 best-effort。
现在只有 reload() 失败会被降级处理；如果数据库写入已经成功，但 emitErrorRulesUpdated() 因 Redis/pubsub 短暂故障抛错，这三个 action 仍会走外层 catch 返回 ok: false。创建路径下这会诱导用户重试并重复创建，更新/删除也会把已提交的变更误报成失败。
可选修正
- await emitErrorRulesUpdated();
+ try {
+ await emitErrorRulesUpdated();
+ } catch (emitError) {
+ logger.warn("[ErrorRulesAction] Failed to broadcast error-rule update", { emitError });
+ }
try {
await errorRuleDetector.reload();
} catch (reloadError) {
logger.warn("[ErrorRulesAction] Failed to reload detector after mutation", { reloadError });
}Also applies to: 321-327, 382-388
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/actions/error-rules.ts` around lines 179 - 187, The call to
emitErrorRulesUpdated() should be made best-effort so transient pub/sub/Redis
failures don't cause the whole action to return ok: false; wrap the
emitErrorRulesUpdated() call in its own try/catch (similar to how
errorRuleDetector.reload() is handled) and on error log a warning with context
(e.g., "[ErrorRulesAction] Failed to emit rules update") but do not rethrow;
apply the same change to the other occurrences referenced (the other
emitErrorRulesUpdated() calls around the 321-327 and 382-388 regions) so emits
never bubble errors back to the caller.
… & timezone-aware timestamps (#1245) Add XLSX usage-logs export (detail sheet + daily/hourly summary sheet), Excel-safe numeric normalization (<=15 sig digits) so SUM() works, and timezone-aware timestamps rendered in the system timezone. Streams batches (no full row retention), async zip, hand-rolled OOXML via fflate. Addressed automated review: OOXML gray125 fill, strict XML 1.0 char filter, invalid-Date guards, sync-xlsx rejection, typed download cast, test cleanup.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3070dd131
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return { | ||
| buffer, | ||
| decoded: false, | ||
| encoding: null, |
There was a problem hiding this comment.
When a client sends a non-empty request with an unsupported Content-Encoding (for example compress), this branch returns decoded: false/encoding: null, so ProxySession leaves the content-encoding header intact, but the normal forwarder path still parses/logs the raw compressed bytes as text and later sends JSON.stringify(session.request.message) rather than session.request.buffer. The upstream then receives an uncompressed JSON body while the original encoding header is still present, causing it to try to decode the wrong bytes; either reject unsupported encodings on preprocessed endpoints or carry an explicit raw-passthrough signal through forwarding.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/actions/usage-logs.ts (1)
177-191:⚠️ Potential issue | 🟠 Major | ⚡ Quick win导出路径漏掉了未完成请求的 live-chain 补全。
这里直接把
findUsageLogsBatch()的结果拿去渲染，但同文件 Line 686-706 的getUsageLogsBatch()会先为未 finalized 的行补_liveChain。现在导出列里的Retry Count仍只看providerChain（src/lib/usage-logs/export/columns.tsLine 30-31），所以导出正在进行中的请求时，重试次数和链路相关字段会落成 0/旧值，和列表页不一致。建议把这里也复用同一套 live-chain merge 逻辑，并让导出列优先读取_liveChain.chain。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/actions/usage-logs.ts` around lines 177 - 191, The export is using findUsageLogsBatch() results directly which misses the live-chain completion applied in getUsageLogsBatch(), causing Retry Count and chain fields to be stale for in-progress requests; before passing batch.logs into buildDetailRowXml or buildCsvRows, run the same live-chain merge logic used by getUsageLogsBatch() (apply the _liveChain augmentation to each log) and update the export columns (export/columns.ts) to prefer reading _liveChain.chain (and related fields) over providerChain so Retry Count reflects the live chain; reference findUsageLogsBatch, getUsageLogsBatch, _liveChain, buildDetailRowXml, buildCsvRows, and the export columns module when making the changes.tests/unit/dashboard-logs-export-progress-ui.test.tsx (1)
31-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick win这里 mock 错了模块路径，当前测试并没有拦住组件实际调用。
UsageLogsFilters导入的是@/lib/api-client/v1/actions/usage-logs，但这里 mock 的是@/actions/usage-logs。这样一来，下面对startUsageLogsExportMock/downloadUsageLogsExportMock的断言并不能证明组件导出链路真的按预期工作。建议修正
-vi.mock("`@/actions/usage-logs`", () => ({ +vi.mock("`@/lib/api-client/v1/actions/usage-logs`", () => ({ startUsageLogsExport: startUsageLogsExportMock, getUsageLogsExportStatus: getUsageLogsExportStatusMock, downloadUsageLogsExport: downloadUsageLogsExportMock, }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/dashboard-logs-export-progress-ui.test.tsx` around lines 31 - 35, The test is mocking the wrong module path so the component still calls the real implementation; update the mock to target the same module imported by UsageLogsFilters (replace "`@/actions/usage-logs`" with "`@/lib/api-client/v1/actions/usage-logs`") so that startUsageLogsExportMock, getUsageLogsExportStatusMock, and downloadUsageLogsExportMock are actually used by the component under test; ensure the vi.mock call references that exact module string and keep the same mock exports (startUsageLogsExport, getUsageLogsExportStatus, downloadUsageLogsExport) so your assertions on startUsageLogsExportMock/downloadUsageLogsExportMock validate the real export flow.
🧹 Nitpick comments (3)
tests/api/v1/usage-logs/usage-logs.test.ts (1)
78-87: ⚡ Quick win把
format也纳入导出状态测试。这次
UsageLogsExportStatus新增了format，但这里的 fixture 和GET /exports/{jobId}断言都没覆盖它；状态接口即使漏返回该字段，这组 API 测试也会继续通过。建议在 mock 里补上format: "csv"，并顺手断言响应里包含它。Also applies to: 270-276
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/api/v1/usage-logs/usage-logs.test.ts` around lines 78 - 87, The export-status test fixture is missing the new format field so add format: "csv" to the object returned by getUsageLogsExportStatusMock.mockResolvedValue(...) and update the corresponding GET /exports/{jobId} response assertion to expect response.body.data.format === "csv" (also apply the same change for the other fixture used around the later test block that calls getUsageLogsExportStatusMock); reference the mock function getUsageLogsExportStatusMock and the route/handler that asserts the export status response to locate the places to change.src/lib/usage-logs/export/xlsx.ts (1)
23-31: ⚡ Quick win改用
@/别名导入本地模块。这一组
./columns、./format、./numeric、./summary不符合仓库当前的导入约定。As per coding guidelines
**/*.{ts,tsx,js,jsx}: Use path alias@/to map to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/usage-logs/export/xlsx.ts` around lines 23 - 31, Update the local imports in xlsx.ts to use the repository path alias instead of relative paths: replace "./columns", "./format", "./numeric" and "./summary" with the corresponding "`@/lib/usage-logs/export/`..." module paths so they resolve from src via the `@/` alias; ensure you keep the same named imports (e.g., isValidDate, toExcelZonedDate, normalizeDecimalForSpreadsheet, createSummaryAccumulator, SUMMARY_HEADERS, SummaryRow, UsageLogsSummary) and adjust any import specifiers to match the aliased module locations.src/lib/usage-logs/export/summary.ts (1)
13-14: ⚡ Quick win改用
@/别名导入本地模块。这里的
./format、./numeric偏离了仓库当前的导入约定，后续移动src/目录时也更难统一重构。As per coding guidelines
**/*.{ts,tsx,js,jsx}: Use path alias@/to map to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/usage-logs/export/summary.ts` around lines 13 - 14, Replace the local relative imports with project path-alias imports: update the imports that bring in isValidDate and toFiniteNumber (currently from "./format" and "./numeric") to use "`@/lib/usage-logs/export/format`" and "`@/lib/usage-logs/export/numeric`" respectively so they follow the repository alias convention and remain stable if src/ is moved; ensure the imported symbols (isValidDate, toFiniteNumber) keep the same names and update any import statements in summary.ts accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/api-client/v1/actions/usage-logs.ts`:
- Around line 85-96: The downloadUsageLogsExport action currently returns only a
Blob which drops Content-Disposition filename and server problem details; update
UsageLogsExportDownloadFile and downloadUsageLogsExport to parse the filename
from the response.headers.get('Content-Disposition') (add filename to the
returned object alongside blob) and, on non-ok responses, read and parse the
response JSON as a problem object (extract errorCode/errorParams and include
them in the thrown/rejected error or returned action result) before passing to
toActionResult so downstream callers (e.g., UsageLogsFilters) receive both the
filename and structured error info.
In `@src/lib/usage-logs/export/xlsx.ts`:
- Around line 243-267: The current assembleUsageLogsXlsx +
detailSheetXml/worksheetXml flow keeps the entire detail sheet in memory because
detailRowsXml: string[] is joined into one big string before zipping; change the
API and implementation to stream the detail sheet instead of materializing it:
replace detailRowsXml: string[] with an async iterable/iterator or chunked
reader (e.g., AsyncIterable<string> or generator) produced by buildDetailRowXml,
update assembleUsageLogsXlsx to create the "xl/worksheets/sheet1.xml" entry as a
streamed/iterable zip entry (emit header, then iterate yielding row chunks, then
footer) rather than calling detailSheetXml(...).join(""), and adjust
detailSheetXml/worksheetXml/buildDetailRowXml to emit/return chunks rather than
a full string so peak memory holds only current chunk + zip buffers. Ensure the
zip writer you use supports streaming entries (or switch to a streaming zip API)
and update callers/tests to provide the new async iterable interface.
---
Outside diff comments:
In `@src/actions/usage-logs.ts`:
- Around line 177-191: The export is using findUsageLogsBatch() results directly
which misses the live-chain completion applied in getUsageLogsBatch(), causing
Retry Count and chain fields to be stale for in-progress requests; before
passing batch.logs into buildDetailRowXml or buildCsvRows, run the same
live-chain merge logic used by getUsageLogsBatch() (apply the _liveChain
augmentation to each log) and update the export columns (export/columns.ts) to
prefer reading _liveChain.chain (and related fields) over providerChain so Retry
Count reflects the live chain; reference findUsageLogsBatch, getUsageLogsBatch,
_liveChain, buildDetailRowXml, buildCsvRows, and the export columns module when
making the changes.
In `@tests/unit/dashboard-logs-export-progress-ui.test.tsx`:
- Around line 31-35: The test is mocking the wrong module path so the component
still calls the real implementation; update the mock to target the same module
imported by UsageLogsFilters (replace "`@/actions/usage-logs`" with
"`@/lib/api-client/v1/actions/usage-logs`") so that startUsageLogsExportMock,
getUsageLogsExportStatusMock, and downloadUsageLogsExportMock are actually used
by the component under test; ensure the vi.mock call references that exact
module string and keep the same mock exports (startUsageLogsExport,
getUsageLogsExportStatus, downloadUsageLogsExport) so your assertions on
startUsageLogsExportMock/downloadUsageLogsExportMock validate the real export
flow.
---
Nitpick comments:
In `@src/lib/usage-logs/export/summary.ts`:
- Around line 13-14: Replace the local relative imports with project path-alias
imports: update the imports that bring in isValidDate and toFiniteNumber
(currently from "./format" and "./numeric") to use
"`@/lib/usage-logs/export/format`" and "`@/lib/usage-logs/export/numeric`"
respectively so they follow the repository alias convention and remain stable if
src/ is moved; ensure the imported symbols (isValidDate, toFiniteNumber) keep
the same names and update any import statements in summary.ts accordingly.
In `@src/lib/usage-logs/export/xlsx.ts`:
- Around line 23-31: Update the local imports in xlsx.ts to use the repository
path alias instead of relative paths: replace "./columns", "./format",
"./numeric" and "./summary" with the corresponding "`@/lib/usage-logs/export/`..."
module paths so they resolve from src via the `@/` alias; ensure you keep the same
named imports (e.g., isValidDate, toExcelZonedDate,
normalizeDecimalForSpreadsheet, createSummaryAccumulator, SUMMARY_HEADERS,
SummaryRow, UsageLogsSummary) and adjust any import specifiers to match the
aliased module locations.
In `@tests/api/v1/usage-logs/usage-logs.test.ts`:
- Around line 78-87: The export-status test fixture is missing the new format
field so add format: "csv" to the object returned by
getUsageLogsExportStatusMock.mockResolvedValue(...) and update the corresponding
GET /exports/{jobId} response assertion to expect response.body.data.format ===
"csv" (also apply the same change for the other fixture used around the later
test block that calls getUsageLogsExportStatusMock); reference the mock function
getUsageLogsExportStatusMock and the route/handler that asserts the export
status response to locate the places to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82ef7179-c378-40b2-b129-2b20e8e6b8cc
📒 Files selected for processing (27)
messages/en/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonmessages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonpackage.jsonsrc/actions/usage-logs.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsxsrc/app/api/v1/resources/usage-logs/handlers.tssrc/lib/api-client/v1/actions/usage-logs.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/usage-logs.tssrc/lib/usage-logs/export/columns.tssrc/lib/usage-logs/export/csv.tssrc/lib/usage-logs/export/format.tssrc/lib/usage-logs/export/numeric.tssrc/lib/usage-logs/export/summary.tssrc/lib/usage-logs/export/xlsx.tstests/api/v1/usage-logs/usage-logs.test.tstests/unit/actions/usage-logs-export-retry-count.test.tstests/unit/actions/usage-logs-export-xlsx.test.tstests/unit/api/v1/api-client-actions.test.tstests/unit/dashboard-logs-export-progress-ui.test.tsxtests/unit/usage-logs/export-csv.test.tstests/unit/usage-logs/export-numeric.test.tstests/unit/usage-logs/export-summary.test.tstests/unit/usage-logs/export-xlsx.test.ts
✅ Files skipped from review due to trivial changes (4)
- messages/zh-CN/dashboard.json
- messages/en/dashboard.json
- src/lib/api-client/v1/openapi-types.gen.ts
- messages/ja/dashboard.json
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- tests/unit/api/v1/api-client-actions.test.ts
| export interface UsageLogsExportDownloadFile { | ||
| blob: Blob; | ||
| } | ||
|
|
||
| export function downloadUsageLogsExport(jobId: string) { | ||
| return toActionResult( | ||
| return toActionResult<UsageLogsExportDownloadFile>( | ||
| fetch(`/api/v1/usage-logs/exports/${encodeURIComponent(jobId)}/download`, { | ||
| credentials: "include", | ||
| }).then(async (response) => { | ||
| if (!response.ok) throw new Error(response.statusText || "Export download failed"); | ||
| return response.text(); | ||
| return { blob: await response.blob() }; | ||
| }) |
There was a problem hiding this comment.
保留下载元信息和结构化错误，不要只返回 Blob。
这里把下载响应压缩成 { blob } 后，服务端新加的 Content-Disposition 文件名和 problem errorCode/errorParams 都丢了。下游 UsageLogsFilters 只能猜文件名，并且失败时拿不到结构化错误码，和本 PR 新建的服务端契约已经脱节了。建议把 filename（至少）一并从响应头解析出来，并在非 2xx 时解析 problem body 后再交给 toActionResult。
建议修正
export interface UsageLogsExportDownloadFile {
blob: Blob;
+ filename: string;
}
export function downloadUsageLogsExport(jobId: string) {
return toActionResult<UsageLogsExportDownloadFile>(
fetch(`/api/v1/usage-logs/exports/${encodeURIComponent(jobId)}/download`, {
credentials: "include",
}).then(async (response) => {
- if (!response.ok) throw new Error(response.statusText || "Export download failed");
- return { blob: await response.blob() };
+ if (!response.ok) {
+ const problem = await response.json().catch(() => null);
+ const error = new Error(problem?.detail || response.statusText || "Export download failed");
+ Object.assign(error, {
+ errorCode: problem?.errorCode,
+ errorParams: problem?.errorParams,
+ });
+ throw error;
+ }
+
+ const disposition = response.headers.get("content-disposition") ?? "";
+ const filename =
+ /filename="([^"]+)"/.exec(disposition)?.[1] ??
+ `usage-logs.${response.headers.get("content-type")?.includes("spreadsheetml") ? "xlsx" : "csv"}`;
+
+ return {
+ blob: await response.blob(),
+ filename,
+ };
})
);
}📝 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.
| export interface UsageLogsExportDownloadFile { | |
| blob: Blob; | |
| } | |
| export function downloadUsageLogsExport(jobId: string) { | |
| return toActionResult( | |
| return toActionResult<UsageLogsExportDownloadFile>( | |
| fetch(`/api/v1/usage-logs/exports/${encodeURIComponent(jobId)}/download`, { | |
| credentials: "include", | |
| }).then(async (response) => { | |
| if (!response.ok) throw new Error(response.statusText || "Export download failed"); | |
| return response.text(); | |
| return { blob: await response.blob() }; | |
| }) | |
| export interface UsageLogsExportDownloadFile { | |
| blob: Blob; | |
| filename: string; | |
| } | |
| export function downloadUsageLogsExport(jobId: string) { | |
| return toActionResult<UsageLogsExportDownloadFile>( | |
| fetch(`/api/v1/usage-logs/exports/${encodeURIComponent(jobId)}/download`, { | |
| credentials: "include", | |
| }).then(async (response) => { | |
| if (!response.ok) { | |
| const problem = await response.json().catch(() => null); | |
| const error = new Error(problem?.detail || response.statusText || "Export download failed"); | |
| Object.assign(error, { | |
| errorCode: problem?.errorCode, | |
| errorParams: problem?.errorParams, | |
| }); | |
| throw error; | |
| } | |
| const disposition = response.headers.get("content-disposition") ?? ""; | |
| const filename = | |
| /filename="([^"]+)"/.exec(disposition)?.[1] ?? | |
| `usage-logs.${response.headers.get("content-type")?.includes("spreadsheetml") ? "xlsx" : "csv"}`; | |
| return { | |
| blob: await response.blob(), | |
| filename, | |
| }; | |
| }) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/api-client/v1/actions/usage-logs.ts` around lines 85 - 96, The
downloadUsageLogsExport action currently returns only a Blob which drops
Content-Disposition filename and server problem details; update
UsageLogsExportDownloadFile and downloadUsageLogsExport to parse the filename
from the response.headers.get('Content-Disposition') (add filename to the
returned object alongside blob) and, on non-ok responses, read and parse the
response JSON as a problem object (extract errorCode/errorParams and include
them in the thrown/rejected error or returned action result) before passing to
toActionResult so downstream callers (e.g., UsageLogsFilters) receive both the
filename and structured error info.
| export interface XlsxParts { | ||
| /** Pre-rendered detail row XML (one entry per data row, row numbers from 2). */ | ||
| detailRowsXml: string[]; | ||
| summary: UsageLogsSummary; | ||
| timezone: string; | ||
| } | ||
|
|
||
| /** | ||
| * Assemble an XLSX workbook (detail sheet + daily/hourly summary sheet) from | ||
| * pre-rendered detail rows and an aggregated summary. Compression runs via | ||
| * fflate's async zip so a large export does not block the event loop. | ||
| */ | ||
| export function assembleUsageLogsXlsx(parts: XlsxParts): Promise<Uint8Array> { | ||
| const files: Record<string, Uint8Array> = { | ||
| "[Content_Types].xml": strToU8(CONTENT_TYPES_XML), | ||
| "_rels/.rels": strToU8(ROOT_RELS_XML), | ||
| "xl/workbook.xml": strToU8(workbookXml(summarySheetName(parts.summary))), | ||
| "xl/_rels/workbook.xml.rels": strToU8(WORKBOOK_RELS_XML), | ||
| "xl/styles.xml": strToU8(STYLES_XML), | ||
| "xl/worksheets/sheet1.xml": strToU8(detailSheetXml(parts.detailRowsXml, parts.timezone)), | ||
| "xl/worksheets/sheet2.xml": strToU8(buildSummarySheet(parts.summary)), | ||
| }; | ||
| return new Promise((resolve, reject) => { | ||
| zip(files, { level: 6 }, (error, data) => (error ? reject(error) : resolve(data))); | ||
| }); |
There was a problem hiding this comment.
这里的“流式”接口实际上仍会把整张明细表一次性留在内存里。
assembleUsageLogsXlsx 要求 detailRowsXml: string[]，随后 detailSheetXml/worksheetXml 又把这些行整体展开并 join("")。这样会同时持有“每行 XML 数组 + 拼接后的整张 sheet XML + zip 输入字节”，大导出时峰值内存仍然随结果大小线性增长，buildDetailRowXml 也无法兑现 Line 168-Line 169 “无需保留整个结果集”的承诺。建议把明细行改成分块/迭代器输入，或直接按流式 zip entry 生成 sheet1.xml。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/usage-logs/export/xlsx.ts` around lines 243 - 267, The current
assembleUsageLogsXlsx + detailSheetXml/worksheetXml flow keeps the entire detail
sheet in memory because detailRowsXml: string[] is joined into one big string
before zipping; change the API and implementation to stream the detail sheet
instead of materializing it: replace detailRowsXml: string[] with an async
iterable/iterator or chunked reader (e.g., AsyncIterable<string> or generator)
produced by buildDetailRowXml, update assembleUsageLogsXlsx to create the
"xl/worksheets/sheet1.xml" entry as a streamed/iterable zip entry (emit header,
then iterate yielding row chunks, then footer) rather than calling
detailSheetXml(...).join(""), and adjust
detailSheetXml/worksheetXml/buildDetailRowXml to emit/return chunks rather than
a full string so peak memory holds only current chunk + zip buffers. Ensure the
zip writer you use supports streaming entries (or switch to a streaming zip API)
and update callers/tests to provide the new async iterable interface.
Make the decompression output cap configurable via MAX_DECOMPRESSED_REQUEST_BYTES env var (default 100MB), and introduce a new MAX_COMPRESSED_REQUEST_BYTES env var (default 10MB) that rejects oversized compressed requests before decompression. This closes a DoS amplification vector in the /v1 and /v1beta proxy paths, which are not governed by proxyClientMaxBodySize. The compressed size check runs pre-auth, limiting CPU/memory usage from unauthenticated clients and bounding the decompression amplification ratio. Refs #1243
Add a tooltip around the Anthropic effort badge in the request details dialog, explaining that the badge shows the verbatim output_config.effort value sent by the client; the proxy does not rename or convert levels. Also add the missing xhigh badge style in anthropic-effort-badge so xhigh is visually distinct between high and max instead of falling back to the gray default.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
The hardcoded 10MB default was rejecting large compressed requests (e.g. 60MB prose, image-heavy payloads) that the plaintext path accepts up to 100MB. Real compression never grows the body, so the limit now tracks the decompressed ceiling, eliminating the asymmetry. The .env.example and code comments document the rationale and the ability to override via MAX_COMPRESSED_REQUEST_BYTES.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58346ce633
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| csvLines.push(...buildCsvRows(batch.logs)); | ||
| if (format === "xlsx" && xlsxSummary) { | ||
| for (const log of batch.logs) { | ||
| xlsxDetailRows.push(buildDetailRowXml(log, xlsxDetailRows.length + 2, timezone)); |
There was a problem hiding this comment.
When an async XLSX export matches more than 1,048,575 usage-log records, this loop keeps assigning all detail rows to the single Usage Logs worksheet (rowNumber starts at 2), which produces row numbers beyond Excel's 1,048,576-row worksheet limit. Large unfiltered exports will therefore generate a workbook Excel cannot open cleanly instead of failing early or splitting across sheets; add a row cap/error for XLSX or roll over to additional worksheets before this limit.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
Release v0.8.4 — aggregates 7 targeted fixes and 1 feature from the
devbranch since v0.8.3, covering proxy robustness (compressed request bodies, benign EPIPE handling), notification scheduling correctness, model price local-first protection, Anthropic API spec compliance, and UI regression fixes.Changes
New Feature: Compressed Request Body Support
content-encoding: zstd(and gzip/deflate/br) on inbound proxy requests — Codex CLI sends zstd-compressed bodies; the proxy now decompresses them before parsing, filtering, and forwarding to upstream. Node.js >= 22.15 is now required (nativenode:zlibzstd support).src/app/v1/_lib/proxy/request-body-codec.ts(220 lines) — decompression pipeline with bomb-guard (100MB cap), single-layer encoding limit, and graceful passthrough for unsupported encodingssrc/app/v1/_lib/proxy/session.ts— integrates decompression intoparseRequestBody, stripscontent-encodingheader post-decode so upstream receives plaintextpackage.jsonengines field set tonode >= 22.15.0, Dockerfiles updated tonode:22-slim/node:trixie-slim, CI uses Node 22Bug Fixes
Don't crash process on benign EPIPE from streaming disconnect
/v1/responsesstream can crash CCH process via uncaughtwrite EPIPEsrc/lib/lifecycle/benign-errors.ts— scoped predicate (EPIPE only, cause-chain traversal), deliberately excludes ECONNRESET/ERR_STREAM_PREMATURE_CLOSE to preserve fail-fast for genuine infrastructure failures (App exits with SIGSEGV under proxy load, causing nginx 502 bursts #1147)src/instrumentation.ts—uncaughtExceptionandunhandledRejectionhandlers now suppress benign broken-pipe errors atwarnlevel instead of callingprocess.exit(1)Fix notification scheduling — master/sub switches and interval edge cases
src/lib/notification/notification-queue.ts— runtime re-check of master + sub switches before sending;removeAllRepeatableJobswith graceful partial-failure handling;intervalToRepeat()correctly maps intervals that don't divide 60 to{every}instead of broken cron expressions;clampIntervalMinutes()normalizes to [1, 1440]src/actions/notifications.ts— scheduleNotifications now called unconditionally (was gated behindNODE_ENV === "production"), internal fail-open handles missing RedisProtect locally-uploaded model prices from cloud auto-sync overwrite (local-first)
src/actions/model-prices.ts—processPriceTableInternalaccepts asourceparameter ("litellm"for auto-sync,"manual"for user uploads); manual-source writes bypass the local-protection skip;uploadPriceTablemarks as"manual"sourcesrc/repository/model-price.ts—upsertModelPriceaccepts configurable source instead of hardcoded"manual"Fix provider group dropdown and model suggestion API client wrappers
src/lib/api-client/v1/actions/providers.ts—getProviderGroupsWithCount()andgetModelSuggestionsByProviderGroup()now wrap raw API responses inActionResult<{ok, data}>viatoActionResult(), matching dashboard consumer expectationssrc/components/ui/tag-input.tsx— auto-open suggestions when async data arrives while input is focused (regression guard for 创建用户无法选择已有供应商分组 #1212)Normalize Anthropic
/v1/modelscreated_atto second precisioncreated_atdoes not conform to the official specification in the /v1/models response of the Anthropic API #1226 —created_atdoes not conform to Anthropic API spec (includes milliseconds)src/app/v1/_lib/models/available-models.ts—normalizeAnthropicTimestamp()strips milliseconds and normalizes timezone offsets; gracefully passes through unparseable timestampsTests Added
tests/unit/proxy/request-body-codec.test.tstests/unit/proxy/session-request-decode.test.tstests/unit/benign-broken-pipe-error.test.tstests/unit/instrumentation-crash-handler.test.tstests/unit/notification/notification-queue.test.tstests/unit/actions/model-prices.test.tstests/unit/api/v1/api-client-actions.test.tstests/unit/proxy/available-models.test.tstests/unit/settings/providers/provider-form-*.test.tsxsrc/components/ui/__tests__/provider-group-tag-input.test.tsxBreaking Changes
node-versionin CI; pull latest Docker images which already use Node 22/24Checklist
Description enhanced by Claude AI
Greptile Summary
This PR bundles v0.8.4 with 7 fixes and 1 feature: compressed request body decompression (zstd/gzip/deflate/br) for inbound proxy requests, EPIPE crash suppression, notification scheduling correctness, local-first model price protection, Anthropic API timestamp compliance, and provider group UI fixes — all backed by ~2,200 lines of new tests.
request-body-codec.ts): single-layer enforcement, 100 MB decompression bomb guard, and header stripping after decode are correctly integrated intoProxySession.fromContext.intervalToRepeat()now maps non-cron-compatible intervals (≥ 60 min or non-factors of 60) to Bull'severymode instead of generating invalid cron expressions like*/60 * * * *; master/sub-switch runtime re-checks prevent stale repeatable jobs from firing after the switch is turned off.processPriceTableInternalaccepts asourceparameter so user-uploaded manual prices are protected from cloud auto-sync overwrites.Confidence Score: 5/5
Safe to merge; all core changes — decompression, EPIPE suppression, notification scheduling, local-first price protection, and UI fixes — are logically correct with solid test coverage.
No correctness or data-integrity defects were found in the changed code paths. The notification interval logic correctly fixes the broken */60 * * * * cron generation. The model-price source-protection logic is sound. The EPIPE suppression is deliberately scoped to write-side errors only. The two flagged items are a silent timezone discard (documented limitation) and an inflated update count for source-only conversions — neither affects correctness of stored data or notifications fired.
No files require special attention, though src/lib/notification/notification-queue.ts is worth a second look regarding the silent timezone drop for intervals >= 60 minutes in bindings that carry an explicit timezone setting.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client as Codex / Client participant Proxy as Proxy (session.ts) participant Codec as request-body-codec.ts participant Guard as Auth Guard participant Upstream as Upstream LLM Client->>Proxy: POST /v1/messages content-encoding: zstd Proxy->>Codec: decodeRequestBody(rawBodyBuffer, zstd) Note over Codec: Check layers <= 1, supported encoding, compressed size <= limit Codec->>Codec: zstdDecompressSync(buffer, maxOutputLength) Note over Codec: Decompression bomb guard (100 MB cap) Codec-->>Proxy: "DecodedRequestBody decoded=true encoding=zstd" Proxy->>Proxy: headers.delete(content-encoding) Proxy->>Guard: ProxySession (plaintext body, no CE header) Guard->>Guard: Auth, filter, billing on plaintext Guard->>Upstream: Forward plaintext body (no content-encoding) Upstream-->>Client: Stream responsePrompt To Fix All With AI
Reviews (5): Last reviewed commit: "fix(proxy): raise compressed body limit ..." | Re-trigger Greptile