fix: Codex session completion must not inject metadata#601
Conversation
📝 Walkthrough概览本PR引入Codex会话ID完成功能,包括数据库架构扩展、多语言消息支持、验证规则、类型定义,以及用于自动填充缺失的会话标识符和缓存管理的核心会话完成器逻辑。 变更
估计代码审查工作量🎯 3 (Moderate) | ⏱️ ~25 minutes 可能相关的PR
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 critical fix and enhancement for handling Codex API requests. Previously, requests were failing due to an unsupported Highlights
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 pull request effectively resolves the issue where metadata was being incorrectly injected into Codex API requests, causing upstream errors. The core of the change is the new codex-session-id-completer, which robustly ensures that both session_id (header) and prompt_cache_key (body) are present and consistent. The implementation includes a smart fallback mechanism that uses request fingerprinting and Redis to generate stable session IDs when they are missing, falling back to a UUID v7 if needed. The changes are well-integrated with a new system setting, UI toggle, and comprehensive unit tests that validate the new logic and confirm that metadata is no longer modified. The code is clean, well-structured, and the addition of a dedicated test coverage script is a great practice.
| | "header_session_id" | ||
| | "header_x_session_id" | ||
| | "body_prompt_cache_key" | ||
| | "body_metadata_session_id" |
There was a problem hiding this comment.
The source type for CodexSessionIdCompletionSpecialSetting includes body_metadata_session_id. The implementation in session-completer.ts correctly avoids reading from metadata, which aligns with this PR's goal to prevent issues with the Codex upstream API. To maintain consistency and avoid confusion, this unused source type should be removed.
There was a problem hiding this comment.
Code Review Summary
This PR adds a new system setting enableCodexSessionIdCompletion to control automatic session ID completion for Codex requests. The implementation is well-structured with proper database migration, type definitions, i18n support, and comprehensive test coverage.
PR Size: XL
- Lines changed: 3444 (3426 additions, 18 deletions)
- Files changed: 28
Note on size: The large line count is primarily due to the auto-generated Drizzle snapshot file (drizzle/meta/0054_snapshot.json ~2388 lines). The actual implementation changes are reasonable in scope.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (Redis failures gracefully fall back to UUID v7 generation with proper logging)
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Comprehensive (dedicated vitest config with 80% threshold)
- Code clarity - Good
Implementation Highlights
-
Session Completer Logic (
src/app/v1/_lib/codex/session-completer.ts):- Properly handles all edge cases: missing headers, missing body fields, invalid session IDs
- Redis fingerprint caching with NX race condition handling
- Graceful fallback to UUID v7 when Redis is unavailable
- Error logging without silent failures
-
Database Migration (
drizzle/0054_tidy_winter_soldier.sql):- Simple ALTER TABLE adding boolean column with sensible default (true)
- Non-breaking change with backward compatibility
-
Test Coverage (
tests/unit/codex/session-completer.test.ts):- 18 test cases covering:
- Header-to-body completion
- Body-to-header completion
- UUID v7 generation
- Redis fingerprint caching and reuse
- Redis NX race conditions
- Redis failure fallback
- Invalid session ID handling
- Tests explicitly verify
metadatais never added/modified (per PR description)
- 18 test cases covering:
-
i18n Support:
- All 5 required languages updated (en, ja, ru, zh-CN, zh-TW)
-
Type Safety:
- New
CodexSessionIdCompletionSpecialSettingtype properly defined SystemSettingsinterface updated with new field- Validation schema includes the new boolean field
- New
Verification Notes
- The PR correctly addresses the stated goal: Codex upstream rejects
metadatafield, so the session completer now only writes tosession_idheader andprompt_cache_keybody field - Tests explicitly assert
expect(body.metadata).toBeUndefined()in relevant cases - The feature is toggleable via system settings with a sensible default (enabled)
Automated review by Claude AI
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 (3)
src/app/v1/_lib/proxy/message-service.ts (1)
24-32: 删除注释中的 emoji(⭐)按规范
**/*.{ts,tsx,js,jsx}代码、注释和字符串中不能出现 emoji,需要移除第 24 行注释中的 "⭐" 字符。修改建议
- // ⭐ 修复模型重定向记录问题: + // 修复模型重定向记录问题: // 由于 ensureContext 在模型重定向之前被调用(guard-pipeline 阶段), // 此时 session.getOriginalModel() 可能返回 null。 // 因此需要在这里提前保存当前模型作为 original_model, // 如果后续发生重定向,ModelRedirector.apply() 会再次调用 setOriginalModel()(幂等性保护)src/lib/config/system-settings-cache.ts (1)
27-48: 避免返回共享的默认对象引用(responseFixerConfig)在兜底返回里直接复用
DEFAULT_SETTINGS.responseFixerConfig可能导致下游误改对象后“污染默认值/跨请求泄露”。建议返回时做一次浅拷贝(该配置目前是扁平结构)。Proposed diff
return { id: 0, siteTitle: "Claude Code Hub", allowGlobalUsageView: false, currencyDisplay: "USD", billingModelSource: "original", verboseProviderError: false, enableAutoCleanup: false, cleanupRetentionDays: 30, cleanupSchedule: "0 2 * * *", cleanupBatchSize: 10000, enableClientVersionCheck: false, enableHttp2: DEFAULT_SETTINGS.enableHttp2, interceptAnthropicWarmupRequests: DEFAULT_SETTINGS.interceptAnthropicWarmupRequests, enableThinkingSignatureRectifier: DEFAULT_SETTINGS.enableThinkingSignatureRectifier, enableCodexSessionIdCompletion: DEFAULT_SETTINGS.enableCodexSessionIdCompletion, enableResponseFixer: DEFAULT_SETTINGS.enableResponseFixer, - responseFixerConfig: DEFAULT_SETTINGS.responseFixerConfig, + responseFixerConfig: { ...DEFAULT_SETTINGS.responseFixerConfig }, createdAt: new Date(), updatedAt: new Date(), } satisfies SystemSettings;Also applies to: 93-114
src/types/special-settings.ts (1)
8-16:source在 special-settings.ts 中包含"body_metadata_session_id",与实际实现不符实际的 Codex 会话完成逻辑(session-completer.ts)中定义的
CodexSessionCompletionSource类型不包含"body_metadata_session_id",只支持header_session_id/header_x_session_id/body_prompt_cache_key/fingerprint_cache/generated_uuid_v7。而 special-settings.ts 中的审计类型定义(line 130)仍然包含此源。虽然 session-extractor.ts 确实会读取
body.metadata.session_id作为第四优先级的备用源,但该值永远不会被转发到 Codex 上游(测试中明确验证expect(body.metadata).toBeUndefined()),所以 PR 目标(禁止 metadata 转发)已正确实现。问题在于类型定义过于宽泛,导致混淆:审计记录的 source 类型应与实际完成流程使用的源保持一致。建议:移除 special-settings.ts 中
CodexSessionIdCompletionSpecialSetting.source的"body_metadata_session_id"项(line 130),确保类型定义与 session-completer.ts 的CodexSessionCompletionSource保持一致。
🤖 Fix all issues with AI agents
In @src/app/v1/_lib/codex/session-completer.ts:
- Around line 216-297: sanitizeCodexRequest is not stripping
requestBody.metadata so upstream receives an unsupported "metadata" param;
update the unsupportedParams list (used inside sanitizeCodexRequest in
request-sanitizer.ts) to include "metadata" so the sanitizer removes any
requestBody.metadata before forwarding, and ensure any tests or callers that
rely on unsupportedParams behavior still pass.
In @tests/unit/codex/session-completer.test.ts:
- Around line 95-116: The test uses a literal UUID in promptCacheKey which
triggers Gitleaks; change the literal to be constructed (e.g., join/split
smaller string constants or concatenate substrings) so no full token appears as
a single string literal; update occurrences referenced by the test "completes
header session_id from body.prompt_cache_key", the promptCacheKey variable, and
any other tests using the same UUID (the other block around the
makeCodexRequestBody usage) to build the ID at runtime instead of embedding the
full UUID.
🧹 Nitpick comments (12)
drizzle/0054_tidy_winter_soldier.sql (1)
1-1: 迁移语义 OK,但需关注大表加列的锁/耗时风险
DEFAULT true NOT NULL对功能默认开启很合理;如果system_settings在生产环境行数较大或数据库版本/参数不确定,建议在发布前确认该 DDL 是否会触发表重写/长锁(必要时采用“先加 nullable 列 + 回填 + 再设 NOT NULL/default”的两步/三步迁移策略)。messages/zh-CN/settings/config.json (1)
43-44: 文案与“禁止 metadata 注入”的目标一致,建议统一术语格式这里的描述只涉及
session_id(请求头)与prompt_cache_key(请求体),没有引入metadata,方向正确;可考虑在中英文里统一 “session id / Session ID” 的大小写与 “UUID v7” 表述风格,避免 UI 文案不一致。tests/unit/usage-doc/opencode-usage-doc.test.tsx (1)
73-73: 测试期望更新合理,但注意 textContent 子串断言的脆弱性从“应包含 OpenAI npm 包”角度看,这个断言更新是合理的;如果后续文档格式化/换行调整导致波动,建议改成提取并解析对应 JSON 片段后再断言字段存在(可选)。
messages/en/settings/config.json (1)
54-55: i18n 键补齐到位,描述与接口约束一致新增的英文文案没有提到
metadata,并明确只围绕 header 的session_id与 body 的prompt_cache_key,与本 PR 目标一致;可选:统一 “session id / Session ID” 的大小写风格以保持全站一致。tests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsx (1)
11-12: 改用测试消息加载器是正确方向;建议确认返回值不共享可变状态这次改动能避免测试依赖文件系统 I/O,整体更稳定。建议确认
loadTestMessages("en")每次返回的 messages 不会被(或不可能被)运行时修改导致跨用例污染;如存在风险,可在 helper 内做深拷贝/冻结。Also applies to: 55-60
messages/ru/settings/config.json (1)
54-55: 新增 RU 文案键值 OK;建议统一术语/字段名呈现方式已按 i18n 方式补齐文案。建议对“Session ID/UUID v7/对话”的俄语表述与其他语言保持一致,并确认 UI 中字段名是否需要保留
session_id/prompt_cache_key原样(代码字段)还是做更友好的解释。messages/zh-TW/settings/config.json (1)
54-55: zh-TW 新增配置文案 OK;留意描述长度在 UI 上的可读性键名与语义清晰,符合 i18n 约束。可顺便确认该段较长描述在设置页是否会出现排版/换行可读性问题(必要时在 UI 组件侧处理)。
messages/ja/settings/config.json (1)
54-55: ja 新增配置文案 OK;建议核对“同一对话内稳定复用”的语义一致性新增键值符合 i18n 结构,内容表达基本到位。建议与其他语言对照确认“同一対話内で安定して再利用”是否与实际实现/产品定义完全一致。
src/repository/message.ts (1)
20-41:specialSettings写入建议保留 null/undefined 语义(若上游类型区分二者)这里用
data.special_settings ?? undefined会把显式null折叠成undefined。如果该仓库对null/undefined有语义区分(例如“显式清空” vs “不传不改”),建议按类型语义直传。基于该仓库对null/undefined语义区分的既有实践(见 learnings),这里最好确认一致性。建议改动(如类型允许)
- specialSettings: data.special_settings ?? undefined, // 特殊设置(审计/展示) + specialSettings: data.special_settings, // 特殊设置(审计/展示)vitest.codex-session-id-completer.config.ts (1)
11-11: 配置文件默认导出可接受,但建议同时提供具名导出以贴合规范你们规范里偏好具名导出;但 Vitest config 通常需要 default export。可以保留 default,同时额外导出一个具名常量供复用/类型引用。
建议修改
-export default defineConfig({ +export const codexSessionIdCompleterVitestConfig = defineConfig({ test: { globals: true, environment: "happy-dom", @@ }, resolve: { alias: { "@": path.resolve(__dirname, "./src"), "server-only": path.resolve(__dirname, "./tests/server-only.mock.ts"), }, }, }); + +export default codexSessionIdCompleterVitestConfig;src/app/v1/_lib/proxy/session-guard.ts (1)
5-5: 新增 import 建议改用@/路径别名,保持项目一致性。当前
../codex/session-completer与“使用@/引用 src 下文件”的规范不一致。建议修改
-import { completeCodexSessionIdentifiers } from "../codex/session-completer"; +import { completeCodexSessionIdentifiers } from "@/app/v1/_lib/codex/session-completer";src/app/v1/_lib/codex/session-completer.ts (1)
150-205: 避免最后一次无 NX 的 SET:存在覆盖并发写入的风险在
SET ... NX失败且二次GET仍为空时(Line 193-194),直接SET(无 NX)可能覆盖另一个并发请求刚写入的值;建议改为再次SET ... NX/ 或直接放弃写入并返回 candidate(让下次请求再尝试),以消除覆盖风险。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (28)
drizzle/0054_tidy_winter_soldier.sqldrizzle/meta/0054_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/settings/config.jsonmessages/ja/settings/config.jsonmessages/ru/settings/config.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/settings/config.jsonpackage.jsonsrc/actions/system-config.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/v1/_lib/codex/session-completer.tssrc/app/v1/_lib/proxy/message-service.tssrc/app/v1/_lib/proxy/session-guard.tssrc/drizzle/schema.tssrc/lib/config/system-settings-cache.tssrc/lib/utils/special-settings.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/message.tssrc/repository/system-config.tssrc/types/special-settings.tssrc/types/system-config.tstests/unit/codex/session-completer.test.tstests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxtests/unit/usage-doc/opencode-usage-doc.test.tsxvitest.codex-session-id-completer.config.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use emoji characters in any code, comments, or string literals
Files:
src/types/system-config.tssrc/types/special-settings.tssrc/drizzle/schema.tssrc/lib/validation/schemas.tssrc/repository/system-config.tssrc/actions/system-config.tstests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxsrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/message-service.tssrc/lib/utils/special-settings.tssrc/repository/_shared/transformers.tstests/unit/codex/session-completer.test.tssrc/app/[locale]/settings/config/page.tsxsrc/repository/message.tsvitest.codex-session-id-completer.config.tssrc/lib/config/system-settings-cache.tssrc/app/v1/_lib/codex/session-completer.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxtests/unit/usage-doc/opencode-usage-doc.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text
Use path alias@/to reference files in./src/directory
Format code with Biome using: double quotes, trailing commas, 2-space indent, 100 character line width
Files:
src/types/system-config.tssrc/types/special-settings.tssrc/drizzle/schema.tssrc/lib/validation/schemas.tssrc/repository/system-config.tssrc/actions/system-config.tstests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxsrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/message-service.tssrc/lib/utils/special-settings.tssrc/repository/_shared/transformers.tstests/unit/codex/session-completer.test.tssrc/app/[locale]/settings/config/page.tsxsrc/repository/message.tsvitest.codex-session-id-completer.config.tssrc/lib/config/system-settings-cache.tssrc/app/v1/_lib/codex/session-completer.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxtests/unit/usage-doc/opencode-usage-doc.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer named exports over default exports
Files:
src/types/system-config.tssrc/types/special-settings.tssrc/drizzle/schema.tssrc/lib/validation/schemas.tssrc/repository/system-config.tssrc/actions/system-config.tstests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxsrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/message-service.tssrc/lib/utils/special-settings.tssrc/repository/_shared/transformers.tstests/unit/codex/session-completer.test.tssrc/app/[locale]/settings/config/page.tsxsrc/repository/message.tsvitest.codex-session-id-completer.config.tssrc/lib/config/system-settings-cache.tssrc/app/v1/_lib/codex/session-completer.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxtests/unit/usage-doc/opencode-usage-doc.test.tsx
src/drizzle/schema.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Modify database schema in
src/drizzle/schema.ts, then runbun run db:generateto generate migrations. Never create SQL migration files manually
Files:
src/drizzle/schema.ts
src/repository/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM for data access in the repository layer
Files:
src/repository/system-config.tssrc/repository/_shared/transformers.tssrc/repository/message.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All new features must have unit test coverage of at least 80%
Files:
tests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxtests/unit/codex/session-completer.test.tstests/unit/usage-doc/opencode-usage-doc.test.tsx
tests/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for unit testing and happy-dom for DOM testing
Files:
tests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxtests/unit/codex/session-completer.test.tstests/unit/usage-doc/opencode-usage-doc.test.tsx
src/app/v1/_lib/proxy/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
The proxy pipeline processes requests through a GuardPipeline with sequential guards: auth, sensitive, client, model, version, probe, session, warmup, requestFilter, rateLimit, provider, providerRequestFilter, messageContext
Files:
src/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/message-service.ts
🧠 Learnings (10)
📚 Learning: 2026-01-05T03:01:39.354Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/types/user.ts:158-170
Timestamp: 2026-01-05T03:01:39.354Z
Learning: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Applied to files:
src/types/system-config.tssrc/types/special-settings.tssrc/drizzle/schema.tssrc/lib/validation/schemas.tssrc/repository/system-config.tssrc/actions/system-config.tssrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/message-service.tssrc/lib/utils/special-settings.tssrc/repository/_shared/transformers.tssrc/repository/message.tssrc/lib/config/system-settings-cache.tssrc/app/v1/_lib/codex/session-completer.ts
📚 Learning: 2026-01-10T17:53:25.066Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to src/drizzle/schema.ts : Modify database schema in `src/drizzle/schema.ts`, then run `bun run db:generate` to generate migrations. Never create SQL migration files manually
Applied to files:
src/drizzle/schema.tsdrizzle/meta/0054_snapshot.json
📚 Learning: 2026-01-05T03:02:14.502Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx:66-66
Timestamp: 2026-01-05T03:02:14.502Z
Learning: In the claude-code-hub project, the translations.actions.addKey field in UserKeyTableRowProps is defined as optional for backward compatibility, but all actual callers in the codebase provide the complete translations object. The field has been added to all 5 locale files (messages/{locale}/dashboard.json).
Applied to files:
messages/zh-TW/settings/config.jsonmessages/ru/settings/config.json
📚 Learning: 2026-01-10T06:19:58.167Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/actions/model-prices.ts:275-335
Timestamp: 2026-01-10T06:19:58.167Z
Learning: Do not modify hardcoded Chinese error messages in Server Actions under src/actions/*.ts as part of piecemeal changes. This is a repo-wide architectural decision that requires a coordinated i18n refactor across all Server Action files (e.g., model-prices.ts, users.ts, system-config.ts). Treat i18n refactor as a separate unified task rather than per-PR changes, and plan a project-wide approach for replacing hardcoded strings with localized resources.
Applied to files:
src/actions/system-config.ts
📚 Learning: 2026-01-10T17:53:25.066Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : All new features must have unit test coverage of at least 80%
Applied to files:
package.jsontests/unit/codex/session-completer.test.tsvitest.codex-session-id-completer.config.tstests/unit/usage-doc/opencode-usage-doc.test.tsx
📚 Learning: 2026-01-10T17:53:25.066Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Use Vitest for unit testing and happy-dom for DOM testing
Applied to files:
package.jsontests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsxvitest.codex-session-id-completer.config.ts
📚 Learning: 2026-01-10T06:20:04.478Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/actions/model-prices.ts:275-335
Timestamp: 2026-01-10T06:20:04.478Z
Learning: In the `ding113/claude-code-hub` repository, Server Actions (files under `src/actions/*.ts`) currently return hardcoded Chinese error messages directly. This is a codebase-wide architectural decision that applies to all action files (e.g., model-prices.ts, users.ts, system-config.ts). Changing this pattern requires a coordinated i18n refactor across all Server Actions, which should be handled as a separate unified task rather than piecemeal changes in individual PRs.
Applied to files:
tests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsx
📚 Learning: 2026-01-10T17:53:25.066Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text
Applied to files:
tests/unit/settings/providers/model-multi-select-custom-models-ui.test.tsx
📚 Learning: 2026-01-10T17:53:25.066Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to src/app/v1/_lib/proxy/**/*.{ts,tsx} : The proxy pipeline processes requests through a GuardPipeline with sequential guards: auth, sensitive, client, model, version, probe, session, warmup, requestFilter, rateLimit, provider, providerRequestFilter, messageContext
Applied to files:
src/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/message-service.ts
📚 Learning: 2026-01-03T09:10:05.684Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 517
File: src/app/v1/_lib/proxy/auth-guard.ts:205-232
Timestamp: 2026-01-03T09:10:05.684Z
Learning: In `src/app/v1/_lib/proxy/auth-guard.ts`, the `extractApiKeyFromHeaders` function is intentionally a standalone simplified version for non-Guard flows. It deliberately does not check for multi-source conflicts (unlike `ProxyAuthenticator.validate`), and the code duplication is intentional to avoid coupling between Guard and non-Guard authentication flows.
Applied to files:
src/app/v1/_lib/proxy/session-guard.ts
🧬 Code graph analysis (6)
src/repository/system-config.ts (1)
src/drizzle/schema.ts (1)
systemSettings(460-518)
src/app/v1/_lib/proxy/session-guard.ts (3)
src/drizzle/schema.ts (1)
systemSettings(460-518)src/lib/config/system-settings-cache.ts (1)
getCachedSystemSettings(58-115)src/app/v1/_lib/codex/session-completer.ts (1)
completeCodexSessionIdentifiers(216-297)
src/repository/message.ts (1)
src/drizzle/schema.ts (1)
messageRequest(276-362)
vitest.codex-session-id-completer.config.ts (2)
scripts/sync-settings-keys.js (1)
path(15-15)scripts/validate-migrations.js (1)
__dirname(18-18)
src/app/v1/_lib/codex/session-completer.ts (2)
src/app/v1/_lib/codex/session-extractor.ts (1)
normalizeCodexSessionId(25-36)src/lib/logger.ts (1)
logger(168-187)
tests/unit/usage-doc/opencode-usage-doc.test.tsx (1)
src/lib/polyfills/file.ts (1)
text(40-42)
🪛 Gitleaks (8.30.0)
tests/unit/codex/session-completer.test.ts
[high] 100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-review
- GitHub Check: pr-description
- GitHub Check: pr-label
- GitHub Check: dev-build-deploy
🔇 Additional comments (22)
package.json (1)
21-21: 脚本命名与覆盖率门禁方向正确,建议在 CI 中显式调用验证可用性新增
test:coverage:codex-session-id-completer与 PR 的 coverage gate 一致;建议确认vitest.codex-session-id-completer.config.ts的文件名/路径在不同环境(本地/CI、bun run)下都能稳定解析,并在 CI 中显式跑一次该脚本避免“新增但未执行”。src/repository/message.ts (1)
43-69:returning补回specialSettings已确认完整映射已确认
toMessageRequest正确映射special_settings至specialSettings(src/repository/message.ts 第 35 行),且类型在MessageRequest接口、Drizzle schema 及 transformer 间保持一致。SpecialSetting类型仅包含可审计的枚举值(参数覆写、响应修复、守卫拦截、缓存设置等),无敏感数据或大体积内容。实现符合预期。vitest.codex-session-id-completer.config.ts (2)
4-41: 覆盖率门槛与隔离/Mock 策略设置合理
happy-dom+ 80% thresholds +mockReset/restoreMocks/clearMocks对“专项覆盖率门禁”很贴合(也符合 learnings 里的约束)。
1-2:__dirname在此仓库中工作正常,无需修改该代码已在仓库的所有 8 个 Vitest 配置文件中使用相同的
__dirname + path.resolve()模式,并在 CI/CD 中成功运行。本项目使用 Bun 作为运行时(而非纯 Node.js ESM),Bun 在所有上下文中原生支持__dirname,无需通过import.meta.url等方式进行转换。Likely an incorrect or invalid review comment.
drizzle/meta/_journal.json (1)
376-389: Drizzle journal 追加条目看起来正确idx 递增、结构与既有 entries 一致。
src/app/[locale]/settings/config/page.tsx (1)
35-49: 新增开关透传到SystemSettingsForm的接线 OK只做了
initialSettings字段补齐,符合预期。src/repository/_shared/transformers.ts (1)
150-183:enableCodexSessionIdCompletion默认值处理合理
?? true对缺省/回填场景友好,并与“默认开启”的语义一致。src/lib/utils/special-settings.ts (1)
31-84: 新增 codex_session_id_completion 的去重 key 生成逻辑一致且稳定序列化字段选择与排序策略和现有分支风格一致,可用于稳定去重。
src/actions/system-config.ts (1)
27-70: UpdateSystemSettingsSchema 已包含 enableCodexSessionIdCompletion 字段,字段透传链路正确schema 验证通过:
enableCodexSessionIdCompletion在src/lib/validation/schemas.ts:757中定义为z.boolean().optional(),与 formData 类型和 updateSystemSettings 调用完全一致,不会出现字段丢弃或验证失败问题。关于代码中的硬编码中文错误消息(第 47 行
"无权限执行此操作"),根据项目 learnings,Server Actions 的 i18n 改造是一个跨文件的统一重构事项,应单独作为专项任务统筹处理,不在单个 PR 中零散改动。src/repository/system-config.ts (1)
135-164:toSystemSettings已正确处理数据库版本兼容性当旧数据库列缺失时,
minimalSelection查询返回的对象中enableCodexSessionIdCompletion为undefined。但toSystemSettings中对该字段已有明确的默认值处理(第 175 行):enableCodexSessionIdCompletion: dbSettings?.enableCodexSessionIdCompletion ?? true,使用
??操作符确保当字段缺失时默认为true,不会出现undefined或类型契约违反的情况。无需修改。src/drizzle/schema.ts (1)
498-502: 新增系统设置列没问题,但请确认已按规范生成迁移。该列
.notNull().default(true)合理,满足“默认开启”的目标。需要确认对应 drizzle 迁移/快照是通过bun run db:generate生成,而非手写 SQL。As per coding guidelines, ...src/types/system-config.ts (2)
47-50: SystemSettings 增加 enableCodexSessionIdCompletion:类型与默认行为一致。
91-93: UpdateSystemSettingsInput 增加可选字段:符合“部分更新”语义。src/lib/validation/schemas.ts (1)
715-775: 确认该字段的入参类型确实是 boolean(否则可能需要 coercion)。你这里用
z.boolean().optional()没问题;但如果该更新入口存在把"true"/"false"当字符串传入的情况,会直接校验失败,建议对调用方/传输层做一次确认。src/app/v1/_lib/proxy/session-guard.ts (2)
51-76: Codex 补全逻辑符合“只写 header.session_id + body.prompt_cache_key,不写 metadata”的目标。另外建议确认
isCodexRequest = Array.isArray(requestMessage.input)的判定在你们所有非 Codex 流量里不会误命中(避免对非 Codex 请求意外写入session_id/prompt_cache_key)。Based on learnings, ...
78-85: 复用本地 systemSettings 做 warmup 判断:减少重复读取,行为更一致。src/app/[locale]/settings/config/_components/system-settings-form.tsx (2)
23-37: 表单状态/提交/回填链路完整,字段透传一致。Also applies to: 64-66, 84-96, 111-113
259-274: i18n 覆盖已验证。 新增的 i18n keyenableCodexSessionIdCompletion和enableCodexSessionIdCompletionDesc已在全部 5 种语言(zh-CN、zh-TW、en、ja、ru)中定义。drizzle/meta/0054_snapshot.json (1)
1952-1958: 确认该 Drizzle snapshot 为自动生成,且默认开启行为符合预期
public.system_settings.enable_codex_session_id_completion设为notNull: true且default: true会在迁移后默认启用该能力;请确认这是期望的默认策略(避免无意扩大行为变更面)。另外该 snapshot 建议仅通过bun run db:generate生成,避免手工编辑导致 schema 漂移(基于 learnings)。tests/unit/codex/session-completer.test.ts (1)
72-343: 覆盖目标很好:明确断言不会新增/改写 metadata,且仅补齐 session_id 与 prompt_cache_key测试用例系统性覆盖了:header/body 补齐、x-session-id 兼容、Redis 复用/异常回退、以及
metadata保持不变/不创建,和 PR 目标一致。Also applies to: 379-555
src/app/v1/_lib/codex/session-completer.ts (2)
207-297: 实现符合目标:不触碰 requestBody.metadata,仅补齐 header/session_id 与 body/prompt_cache_key
completeCodexSessionIdentifiers只会写入headers.session_id/headers.x-session-id/requestBody.prompt_cache_key,没有任何metadata注入或改写路径;并且“仅补缺失字段、不覆盖已有值”的策略清晰且幂等。
150-205: ioredis v5 API 用法验证正确代码中对
redis.status === "ready"的检查、redis.set(key, value, "EX", ttlSeconds, "NX")的参数顺序以及返回值语义均符合 ioredis v5 规范。SET 命令在 NX 条件未满足时返回 null(Redis nil 映射),代码中的setResult === "OK"判断恰当地处理了成功和失败场景。
| test("completes header session_id from body.prompt_cache_key", async () => { | ||
| const { completeCodexSessionIdentifiers } = await import( | ||
| "@/app/v1/_lib/codex/session-completer" | ||
| ); | ||
|
|
||
| const promptCacheKey = "019b82ff-08ff-75a3-a203-7e10274fdbd8"; | ||
| const headers = new Headers(); | ||
| const body = makeCodexRequestBody({ prompt_cache_key: promptCacheKey }); | ||
|
|
||
| const result = await completeCodexSessionIdentifiers({ | ||
| keyId: 1, | ||
| headers, | ||
| requestBody: body, | ||
| userAgent: "codex_cli_rs/0.50.0", | ||
| }); | ||
|
|
||
| expect(result.applied).toBe(true); | ||
| expect(result.sessionId).toBe(promptCacheKey); | ||
| expect(headers.get("session_id")).toBe(promptCacheKey); | ||
| expect(body.prompt_cache_key).toBe(promptCacheKey); | ||
| expect(body.metadata).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
处理 Gitleaks 命中:测试里的 UUID 字符串可能导致 CI 失败
静态扫描在 Line 100(以及 Line 349)把该 UUID 误判成“Generic API Key”的概率很高,但如果仓库启用了 gitleaks gate,这会直接阻断合并。建议把该值改为拼接/拆分常量,避免以“完整 token”形态出现在源码中。
建议修改(避免完整 UUID 字面量)
+const EXAMPLE_UUID_V7 = "019b82ff-08ff-75a3-a203-" + "7e10274fdbd8";
+
test("completes header session_id from body.prompt_cache_key", async () => {
@@
- const promptCacheKey = "019b82ff-08ff-75a3-a203-7e10274fdbd8";
+ const promptCacheKey = EXAMPLE_UUID_V7;
@@
test("handles Redis NX race by re-reading existing value", async () => {
@@
- const existing = "019b82ff-08ff-75a3-a203-7e10274fdbd8";
+ const existing = EXAMPLE_UUID_V7;Also applies to: 344-377
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In @tests/unit/codex/session-completer.test.ts around lines 95 - 116, The test
uses a literal UUID in promptCacheKey which triggers Gitleaks; change the
literal to be constructed (e.g., join/split smaller string constants or
concatenate substrings) so no full token appears as a single string literal;
update occurrences referenced by the test "completes header session_id from
body.prompt_cache_key", the promptCacheKey variable, and any other tests using
the same UUID (the other block around the makeCodexRequestBody usage) to build
the ID at runtime instead of embedding the full UUID.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
* PR: 修复 Edge Runtime 下 `process.once` 构建告警(AsyncTaskManager 导入链) (#589) * fix: skip async task manager init on edge * fix: avoid static async task manager import * test: cover edge runtime task scheduling * chore: document edge runtime process.once fix * chore: record edge runtime warning baseline * fix: drop NEXT_PHASE and lazy-init async task manager * test: isolate NEXT_RUNTIME in cloud price sync tests * docs: stabilize edge process.once repro baseline * docs: make rollback instructions hashless * docs: add grep checklist for edge warning audit * chore: run regression gate and align docs * test: cover edge runtime guard on register * Update src/lib/async-task-manager.ts 补充 NEXT_PHASE === "phase-production-build" 检查 Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * chore: format code (fix-edge-runtime-process-once-bee7e19) * PR:i18n settings 拆分与翻译质量门禁 (#588) * refactor(i18n): split settings json into smaller files * refactor(i18n): load settings from split module * refactor(i18n): remove legacy settings.json * chore(i18n): update sync-settings-keys for split layout * test(i18n): add split settings guards * chore: align biome schema version * chore(i18n): document messages loading contract * chore(i18n): add settings split verification notes * chore: format code (refactor-i18n-split-settings-3f48fec) * chore: fix i18n request formatting * chore: format code (refactor-i18n-split-settings-a1eff62) * fix: replace settings placeholder translations * chore: verify settings sync script is idempotent * test: run i18n settings split guards * test: add audit for zh-CN placeholder settings strings * chore: apply biome formatting * chore: document manual i18n settings verification * fix: translate all providers filter in ja * fix: translate all providers filter in zh-TW * fix: translate providers section copy in zh-TW * fix: translate providers section copy in ja * feat: extend placeholder audit output * feat: add allowlist for placeholder audit * docs: define i18n translation quality rules * chore: add i18n audit fail commands * docs: add i18n PR checklist * chore: format i18n audit tests * fix: translate dashboard placeholders * fix: translate myUsage placeholders * fix: enforce locale-specific parentheses * fix: start translating provider form strings * fix: translate provider form strings * fix: translate provider guide content * test: add ja dashboard parentheses guard * test: add zh-TW dashboard parentheses guard * test: add zh-TW myUsage parentheses guard * chore: translate ja provider form strings * chore: translate zh-TW provider form strings * chore: translate ja providers guide * chore: translate zh-TW providers guide * chore: refine zh-TW dashboard strings * chore: translate ja providers strings * chore: translate zh-TW providers strings * chore: refine zh-TW api test strings * chore: translate zh-TW settings small modules * chore: translate ja settings small modules * chore: clear i18n placeholders in settings * chore: format code (refactor-i18n-split-settings-2437d19) * test: fix biome formatting in i18n test * chore: verify Biome lint gate (I18NE-030) * chore: add messages emoji audit script (I18NE-010) * fix: remove emoji from messages warnings (I18NE-040) * test: add messages no-emoji audit gate (I18NE-050) * docs: add zh-CN i18n docs (I18NE-020) * docs: add messages no-emoji rule (I18NE-060) * chore: run full regression checks (I18NE-070) * docs: add i18n PR evidence template (I18NE-080) * fix: make messages no-emoji audit path-sep safe * docs: add bun alias for messages no-emoji audit * fix: detect keycap and flag emoji sequences in i18n message audits * fix(provider): allow removing custom whitelisted models (#592) (#593) * fix(rectifier): detect 'signature: Field required' error and trigger rectifier (#594) - Extend detectThinkingSignatureRectifierTrigger to match 'signature: Field required' - Add Rule 72 for friendly error message when signature field is missing - Add comprehensive test cases for the new detection logic This fixes an issue where switching from non-Anthropic to Anthropic channels with thinking blocks missing signature fields would fail without proper handling. * feat(users): increase provider group length to 200 (#591) close #590 * feat(usage-doc): update OpenCode config example with GPT-5.2 and Gemini v1beta (#597) - Add OpenAI GPT-5.2 model configuration with reasoningEffort options - Add GPT-5.2-small variant using medium reasoning effort - Fix Gemini baseURL to use /v1beta endpoint - Update i18n strings to reflect different baseURLs per provider * feat: auto-complete Codex session identifiers (#599) * fix: Codex session completion must not inject metadata (#601) * feat: auto-complete Codex session identifiers * fix: avoid Codex metadata injection --------- Co-authored-by: YangQing-Lin <56943790+YangQing-Lin@users.noreply.github.com> Co-authored-by: Hwwwww-dev <47653238+Hwwwww-dev@users.noreply.github.com>
Why\nCodex upstream rejects the request body field
metadata(400: Unsupported parameter: metadata). The session completion feature must only write the supported Codex fields: headersession_idand bodyprompt_cache_key.\n\n## What\n- Stop writing/creatingmetadatain Codex request bodies\n- Keep completion strictly to:\n - request header:session_id(and preserve compatibility with inboundx-session-id)\n - request body:prompt_cache_key\n\n## Tests\n- Updated unit tests to assertmetadatais never added/modified\n- Coverage gate (>=80%):bun run test:coverage:codex-session-id-completerGreptile Overview
Greptile Summary
This PR successfully addresses the Codex upstream requirement by implementing a session ID completion system that strictly avoids writing to the
metadatafield, which Codex rejects with a 400 error.Key Changes
Core Implementation (session-completer.ts - 297 new lines)
completeCodexSessionIdentifiers()that ensures both required Codex fields are present:session_id(+x-session-idfor compatibility)prompt_cache_keymetadata- verification confirmed through code inspection and explicit test assertionsIntegration & Configuration
ProxySessionGuardbefore session trackinginputarray field)enableCodexSessionIdCompletion(default: true) allows togglingTesting & Quality
metadatais never added or modifiedConsistency Across Stack
All 28 files properly updated across the full stack, including:
Minor Issue Found
One style comment left regarding type definition inconsistency in
special-settings.ts- thesourceunion includesbody_metadata_session_idwhich the completer never returns (it only reads, never writes metadata). This is a minor type hygiene issue with no runtime impact.Confidence Score: 5/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant Client as Codex Client participant Guard as ProxySessionGuard participant Completer as CodexSessionCompleter participant Redis as Redis Cache participant Upstream as Codex Upstream Client->>Guard: Request (may have session_id OR prompt_cache_key OR neither) Guard->>Guard: Check enableCodexSessionIdCompletion Guard->>Guard: Detect Codex request (has input array) Guard->>Completer: completeCodexSessionIdentifiers() alt Both session_id header AND prompt_cache_key body exist Completer->>Completer: Validate both values Completer->>Guard: Return (applied: false, action: "none") else One identifier exists Completer->>Completer: Copy existing value to missing field Completer->>Completer: Ensure both session_id and x-session-id headers present Completer->>Guard: Return (applied: true, action: "completed_missing_fields") else Neither identifier exists Completer->>Completer: Calculate fingerprint (keyId + IP + UA + messageHash) Completer->>Redis: Check if fingerprint exists in cache alt Fingerprint found in Redis Redis->>Completer: Return cached session_id Completer->>Guard: Return (applied: true, action: "reused_fingerprint_cache") else Fingerprint not found OR Redis unavailable Completer->>Completer: Generate UUID v7 Completer->>Redis: Store fingerprint->session_id mapping (if Redis available) Completer->>Guard: Return (applied: true, action: "generated_uuid_v7") end Completer->>Completer: Set session_id and x-session-id headers Completer->>Completer: Set prompt_cache_key in body end Guard->>Guard: Record completion action in special settings Guard->>Upstream: Forward request with completed identifiers Note over Guard,Upstream: Body NEVER contains metadata field Note over Guard,Upstream: Only session_id (header) and prompt_cache_key (body)