fix: Codex session_id fallback to prompt_cache_key#521
Conversation
📝 Walkthrough演练引入新的会话源类型"body_prompt_cache_key"用于Codex会话ID提取。实现提取逻辑,更新优先级顺序,将body.prompt_cache_key置于metadata.session_id之上。为新源和优先级回退行为添加全面测试覆盖。 更改
预估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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! 此拉取请求旨在通过引入 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
本次 PR 的改动目标明确,即在 Codex 会话 ID 提取逻辑中增加 prompt_cache_key 作为备选方案,以提高会话粘性的稳定性。代码实现清晰、直接,并且与现有逻辑融合得很好。新的提取优先级(header > prompt_cache_key > metadata.session_id)已正确实现。
值得称赞的是,您为新功能和边缘场景(如优先级、无效值回退)补充了全面的单元测试,这大大保证了代码的质量和可靠性。
整体来看,这是一次高质量的修复,代码风格一致,逻辑严谨。我没有发现任何需要修改的问题。
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/v1/_lib/codex/session-extractor.ts (1)
91-99: 实现逻辑正确,但建议统一注释语言。提取逻辑正确实现了
prompt_cache_key的验证和优先级处理,位置恰当地插入在 header 检查之后、metadata 检查之前。Line 91 的中文注释与代码库中其他英文注释风格不一致,建议改为英文以保持一致性。
🔎 建议修改为英文注释
- // 当请求头未提供 session_id 时,优先尝试使用 prompt_cache_key 作为稳定的会话标识 + // When the request header does not provide session_id, prefer using prompt_cache_key as a stable session identifier const bodyPromptCacheKey = normalizeCodexSessionId(requestBody.prompt_cache_key);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (2)
src/app/v1/_lib/codex/__tests__/session-extractor.test.tssrc/app/v1/_lib/codex/session-extractor.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation in all code files
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use double quotes for strings instead of single quotes
Use trailing commas in multi-line structures
Enforce maximum line length of 100 characters
Use path alias@/*to reference files from./src/*directory
**/*.{ts,tsx,js,jsx}: Use Biome for linting and formatting with 2-space indent, double quotes, trailing commas, and 100 character max line length
Use path alias@/*to reference files in./src/*directory
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
src/app/v1/_lib/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Guard pipeline must execute in order: ProxyAuthenticator, SensitiveWordGuard, VersionGuard, ProxySessionGuard, ProxyRateLimitGuard, ProxyProviderResolver
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode for type safety
Use readonly or const assertions for immutable data structures
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Hash API keys using SHA-256 before storing in database, never store plaintext keys
Mask API keys and sensitive data in application logs
Validate required environment variables at startup with clear error messages
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
src/app/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement Content-Security-Policy headers for XSS prevention
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
src/app/v1/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Hono router for ultrafast, lightweight routing in proxy endpoints
Files:
src/app/v1/_lib/codex/session-extractor.tssrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Files:
src/app/v1/_lib/codex/__tests__/session-extractor.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Ensure test database names contain 'test' keyword for safety validation
Files:
src/app/v1/_lib/codex/__tests__/session-extractor.test.ts
🧬 Code graph analysis (1)
src/app/v1/_lib/codex/__tests__/session-extractor.test.ts (1)
src/app/v1/_lib/codex/session-extractor.ts (1)
extractCodexSessionId(66-124)
🪛 Gitleaks (8.30.0)
src/app/v1/_lib/codex/__tests__/session-extractor.test.ts
[high] 25-25: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 49-49: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 61-61: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 116-116: 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). (7)
- GitHub Check: pr-description
- GitHub Check: Docker Build Test
- GitHub Check: 🌐 API Tests
- GitHub Check: check-codex-status
- GitHub Check: pr-review
- GitHub Check: pr-label
- GitHub Check: dev-build-deploy
🔇 Additional comments (7)
src/app/v1/_lib/codex/__tests__/session-extractor.test.ts (5)
25-25: 测试数据合理,静态分析误报可忽略。添加
prompt_cache_key到测试数据中是正确的,确保了即使存在 body 字段时,header 依然具有最高优先级。注意:Gitleaks 将此 UUID 标记为 API key 是误报,这是标准的测试数据,不涉及安全风险。
48-58: 新测试覆盖了 prompt_cache_key 提取路径。测试逻辑正确,验证了从
body.prompt_cache_key提取会话 ID 的基本功能。注意:Line 49 的静态分析警告是误报,UUID 是测试数据而非真实 API key。
60-71: 优先级测试逻辑正确。此测试确保
prompt_cache_key在优先级链中高于metadata.session_id,符合 PR 目标和实现逻辑。注意:Line 61 的静态分析警告是误报。
73-83: 边界测试覆盖了无效值降级逻辑。测试确保了当
prompt_cache_key不符合验证规则时(长度 < 21),系统正确降级到metadata.session_id,体现了健壮的错误处理。
116-116: 完善了优先级测试的完整性。在所有可能的会话来源都存在时,测试验证了 header session_id 具有最高优先级,覆盖了新增的
prompt_cache_key字段。注意:Line 116 的静态分析警告是误报。
src/app/v1/_lib/codex/session-extractor.ts (2)
6-6: 类型定义正确添加了新的会话来源。
"body_prompt_cache_key"的命名风格与现有来源一致,TypeScript 类型系统将确保此值在整个代码库中的正确使用。
60-62: 优先级文档准确更新。注释清晰地说明了新的提取优先级顺序,将
body.prompt_cache_key置于第 3 位,高于body.metadata.session_id,符合 PR 目标。
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: S
- Lines changed: 55
- Files changed: 2
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- PR
#521classified as S (55 lines changed, 2 files) and labeledsize/S. - Reviewed the diff in
src/app/v1/_lib/codex/session-extractor.tsandsrc/app/v1/_lib/codex/__tests__/session-extractor.test.ts; no significant issues found within the changed lines. - Submitted the required summary review comment on the PR via
gh pr review.
背景
prompt_cache_key,且通常与请求头里的session_id一致。session_id/x-session-id、bodymetadata.session_id、bodyprevious_response_id。当 header 缺失时,会导致会话粘性不稳定。prompt_cache_key作为回退选项。改动
prompt_cache_key纳入 Codex session_id 提取链路(仅在 header 未命中时启用,优先级高于metadata.session_id)。提取优先级
headers["session_id"]headers["x-session-id"]body.prompt_cache_key← 新增body.metadata.session_idbody.previous_response_id(withcodex_prev_prefix)验证
Related:
prompt_cache_keyfallback to the session extraction chain implemented in feat(codex): implement session isolation for concurrent Codex requests (#430) #434Enhanced by Claude Code