refactor(proxy): remove format converters and enforce same-format routing#709
refactor(proxy): remove format converters and enforce same-format routing#709
Conversation
…ting BREAKING CHANGE: Cross-format conversion is no longer supported. Requests must be routed to providers with matching API formats. - Delete all converters (claude-to-openai, openai-to-claude, codex-*, gemini-cli-*) - Remove Codex CLI adapter, instruction injection, and request sanitizer - Simplify ProxyForwarder to pass-through without format transformation - Update provider-selector to enforce format compatibility - Remove ResponseTransformer conversion logic from response-handler - Clean up session-extractor to remove Codex-specific handling - Delete related test files for removed functionality Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…rovider forms - Remove legacy pool joining and instruction strategy UI controls - Clean up i18n messages for removed provider form fields (all 5 languages) - Update provider actions and form context/types - Remove unused routing section options - Update related test mocks Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Remove outdated claims about format conversion and Codex CLI injection - Clarify that proxy enforces same-format routing with no cross-format conversion - Add format-compatibility unit tests for provider-selector Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
📝 Walkthrough总体概述本 PR 移除了对 Codex 功能和 Claude 调度池的支持,包括删除相关格式转换器、CLI 适配器和配置选项,同时更新了多语言文档和简化了代理转发逻辑。 变更内容
预估代码审查工作量🎯 3 (中等) | ⏱️ ~25 分钟 可能相关的 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the proxy's architecture by removing all cross-format conversion logic and enforcing a strict same-format routing policy. The change streamlines the system by eliminating complex conversion layers and their associated configurations, ensuring that client requests are routed only to providers that natively support their API format. This simplifies maintenance, reduces potential points of failure, and clarifies the expected behavior for API consumers. 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 is an excellent and significant refactoring that greatly simplifies the proxy's architecture by removing the complex API format conversion layer. The changes are consistently applied across the entire codebase, from backend logic and database schemas to UI components and internationalization files. Removing thousands of lines of code is a major win for maintainability and reduces the surface area for bugs.
The core logic change to enforce strict same-format routing is implemented cleanly in the ProxyProviderResolver, and the new test suite for format compatibility provides strong confidence in this new behavior. The extensive cleanup of related code, including UI elements, configuration options, and unused variables, is also well-executed.
Overall, this is a high-quality pull request that makes a substantial improvement to the project. I have no specific change requests.
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/v1/_lib/proxy/provider-selector.ts (1)
747-756:⚠️ Potential issue | 🟠 Major会话复用路径缺少格式兼容校验,可能绕过“同格式路由”。
这里新增了格式过滤,但findReusable未复用同一规则,旧会话绑定可能导致跨格式复用。建议在复用前补充checkFormatProviderTypeCompatibility检查。建议补丁
@@ // 检查模型支持(使用新的模型匹配逻辑) const requestedModel = session.getOriginalModel(); + if ( + session.originalFormat && + !checkFormatProviderTypeCompatibility(session.originalFormat, provider.providerType) + ) { + logger.debug("ProviderSelector: Session provider format mismatch, reject reuse", { + sessionId: session.sessionId, + providerId: provider.id, + providerType: provider.providerType, + originalFormat: session.originalFormat, + }); + return null; + } if (requestedModel && !providerSupportsModel(provider, requestedModel)) { logger.debug("ProviderSelector: Session provider does not support requested model", { sessionId: session.sessionId,
🤖 Fix all issues with AI agents
In `@src/app/v1/_lib/codex/__tests__/session-extractor.test.ts`:
- Around line 18-22: The test uses UUID-like strings for prompt_cache_key which
trigger gitleaks; update the test to replace those values with a clear
non-secret placeholder constant (e.g., TEST_PROMPT_CACHE_KEY) and reuse that
constant across all affected cases (lines around the other occurrences) so
extractCodexSessionId is exercised the same way without using UUID-like keys;
locate and modify the usages in the session-extractor tests that call
extractCodexSessionId and replace prompt_cache_key values with the shared
placeholder constant.
🧹 Nitpick comments (5)
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx (1)
51-58: 考虑移除未使用的 statusCode 透传
目前仅改名为_statusCode以规避未使用提示,但该 prop 在组件内完全未用到。若上游不再需要,建议从LogicTraceTabProps与调用处移除,减少无效数据传递与接口噪音。src/app/[locale]/dashboard/availability/_components/endpoint/probe-terminal.tsx (2)
4-4: 导入了未使用的图标组件
CheckCircle2、XCircle和AlertCircle已导入,并在levelConfig中分配给icon属性,但这些图标从未在 JSX 中渲染。这似乎是移除动态 Icon 渲染后遗留的未清理代码。♻️ 建议移除未使用的导入
-import { AlertCircle, CheckCircle2, Download, XCircle } from "lucide-react"; +import { Download } from "lucide-react";
43-65:levelConfig中的icon属性已成为死代码每个级别配置中的
icon属性不再被使用,因为日志行渲染中已移除了动态 Icon 组件的使用。建议移除这些未使用的属性以保持代码整洁。♻️ 建议移除未使用的 icon 属性
const levelConfig = { success: { - icon: CheckCircle2, label: "OK", color: "text-emerald-500", bgColor: "bg-emerald-500/5", borderColor: "border-l-emerald-500", }, error: { - icon: XCircle, label: "FAIL", color: "text-rose-500", bgColor: "bg-rose-500/5", borderColor: "border-l-rose-500", }, warn: { - icon: AlertCircle, label: "WARN", color: "text-amber-500", bgColor: "bg-amber-500/5", borderColor: "border-l-amber-500", }, };src/app/[locale]/settings/prices/_components/price-list.tsx (1)
71-74: 移除未使用的 locale hook 以减少无效依赖
_locale未被使用,保留useLocale()会增加无意义的 hook 调用与依赖。建议直接移除该 hook 与导入。建议修改(移除未使用的 useLocale)
-import { useLocale, useTimeZone, useTranslations } from "next-intl"; +import { useTimeZone, useTranslations } from "next-intl"; @@ - const _locale = useLocale();src/app/v1/_lib/codex/__tests__/session-extractor.test.ts (1)
1-3: 测试文件位置与源码相邻约定不一致
当前测试位于 tests 目录,建议移动到 src/app/v1/_lib/codex/session-extractor.test.ts 以满足源文件旁边放置的约定。As per coding guidelines, 源码相邻测试应放在 src/**/*.test.ts 旁边.
| const result = extractCodexSessionId(new Headers({ "x-session-id": headerSessionId }), { | ||
| prompt_cache_key: "019b82ff-08ff-75a3-a203-7e10274fdbd8", | ||
| metadata: { session_id: "sess_aaaaaaaaaaaaaaaaaaaaa" }, | ||
| previous_response_id: "resp_123456789012345678901", | ||
| }); |
There was a problem hiding this comment.
替换触发 gitleaks 的 prompt_cache_key 测试值
静态扫描已将这些 UUID 样式字符串识别为 generic-api-key,可能导致 CI 阻断或误报。建议改成明显的测试占位值并复用常量。
建议修改
import { describe, expect, test } from "vitest";
import { extractCodexSessionId } from "../session-extractor";
+const PROMPT_CACHE_KEY = "pcache_test_123456789012345678901";
+
describe("Codex session extractor", () => {
test("extracts from header session_id", () => {
const headerSessionId = "sess_123456789012345678901";
@@
test("extracts from header x-session-id", () => {
const headerSessionId = "sess_123456789012345678902";
const result = extractCodexSessionId(new Headers({ "x-session-id": headerSessionId }), {
- prompt_cache_key: "019b82ff-08ff-75a3-a203-7e10274fdbd8",
+ prompt_cache_key: PROMPT_CACHE_KEY,
metadata: { session_id: "sess_aaaaaaaaaaaaaaaaaaaaa" },
previous_response_id: "resp_123456789012345678901",
});
@@
test("extracts from body prompt_cache_key", () => {
- const promptCacheKey = "019b82ff-08ff-75a3-a203-7e10274fdbd8";
+ const promptCacheKey = PROMPT_CACHE_KEY;
const result = extractCodexSessionId(new Headers(), { prompt_cache_key: promptCacheKey });
@@
test("prompt_cache_key has higher priority than metadata.session_id", () => {
- const promptCacheKey = "019b82ff-08ff-75a3-a203-7e10274fdbd8";
+ const promptCacheKey = PROMPT_CACHE_KEY;
const metadataSessionId = "sess_123456789012345678903";
const result = extractCodexSessionId(new Headers(), {
prompt_cache_key: promptCacheKey,
metadata: { session_id: metadataSessionId },
});
@@
const result = extractCodexSessionId(
new Headers({
session_id: sessionIdFromHeader,
"x-session-id": xSessionIdFromHeader,
}),
{
- prompt_cache_key: "019b82ff-08ff-75a3-a203-7e10274fdbd8",
+ prompt_cache_key: PROMPT_CACHE_KEY,
metadata: { session_id: sessionIdFromBody },
previous_response_id: previousResponseId,
}
);Also applies to: 39-41, 49-52, 98-101
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In `@src/app/v1/_lib/codex/__tests__/session-extractor.test.ts` around lines 18 -
22, The test uses UUID-like strings for prompt_cache_key which trigger gitleaks;
update the test to replace those values with a clear non-secret placeholder
constant (e.g., TEST_PROMPT_CACHE_KEY) and reuse that constant across all
affected cases (lines around the other occurrences) so extractCodexSessionId is
exercised the same way without using UUID-like keys; locate and modify the
usages in the session-extractor tests that call extractCodexSessionId and
replace prompt_cache_key values with the shared placeholder constant.
There was a problem hiding this comment.
Code Review Summary
This PR successfully removes ~9000 lines of format converter code and enforces strict same-format routing. The review found no significant issues requiring fixes before merge.
PR Size: XL
- Lines changed: 9591 (374 additions, 9217 deletions)
- Files changed: 85
This is an extra-large PR focused on code deletion and architectural simplification.
Issues Found
None - No critical, high, or medium priority issues identified.
All code changes are clean deletions or necessary adaptations to the converter removal. The new test coverage validates format compatibility enforcement.
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
Analysis Summary
What Changed:
- Deleted ~8800 lines of format converters (claude↔openai, codex↔*, gemini-cli↔*)
- Deleted Codex CLI adapter components (~990 lines)
- Simplified ProxyForwarder, ProxyResponseHandler, format-mapper.ts
- Added format compatibility enforcement in provider-selector.ts (checkFormatProviderTypeCompatibility function at line 192)
- Added comprehensive test coverage for format matching (289 lines)
- Updated UI to remove joinClaudePool and codexInstructionsStrategy fields
Architectural Decision:
The PR enforces strict same-format routing where requests must be routed to providers with matching API formats:
- Claude API → claude/claude-auth providers only
- OpenAI API → openai-compatible providers only
- Codex Response API → codex providers only
- Gemini Direct → gemini providers only
- Gemini CLI → gemini-cli providers only
Code Quality:
- Clean deletions with no orphaned references
- Proper error context in filteredProviders tracking (format_type_mismatch reason added)
- Comprehensive test coverage validates all format combinations
- No silent failures - incompatible providers are explicitly filtered and logged
Test Coverage:
New test file validates:
- All format/providerType compatibility rules
- Proper rejection of incompatible providers
- Correct filteredProviders context tracking
- Edge cases (no compatible providers, multiple incompatibilities)
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: XL
- Lines changed: 9591
- Files changed: 85
- Split suggestions: Consider separating into (1) proxy pipeline refactor (forwarder/response-handler/format-mapper), (2) mass deletions of converters + codex adapter, (3) UI + i18n cleanup, (4) docs + tests.
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.
- Identified PR
#709(refactor(proxy): remove format converters and enforce same-format routing) from the Actionspull_request_targetevent payload. - Calculated PR size as XL (
374additions +9217deletions =9591lines changed;85files changed) and applied labelsize/XL. - Completed a diff-scoped review across the requested perspectives and submitted a PR review comment with no issues meeting the reporting threshold (no inline review comments posted).
Summary
claude-to-openai,openai-to-claude,codex-*,gemini-cli-*converters (~8800 lines)joinClaudePoolandcodexInstructionsStrategyfrom provider forms (all 5 languages)Problem
Related Issues:
The codebase contained ~8800 lines of format converter implementations (
src/app/v1/_lib/converters/) that were:Solution
Architectural Decision: Enforce strict same-format routing policy.
Requests must be routed to providers with matching API formats:
Rationale:
Changes
Core Proxy Changes
src/app/v1/_lib/converters/**(all converter implementations: ~8800 lines)claude-to-openai,openai-to-claudecodex-to-claude,codex-to-openai,claude-to-codex,openai-to-codexgemini-cli-to-claude,gemini-cli-to-openairegistry.ts,tool-name-mapper.ts, converter typeschat-completions-handler.ts(214 lines)codex-cli-adapter.ts(92 lines)constants/codex-cli-instructions.ts(354 lines)constants/codex-instructions.ts(193 lines)utils/request-sanitizer.ts(137 lines)ProxyForwarder- removed format transformation logic (src/app/v1/_lib/proxy/forwarder.ts:~98 lines removed)ProxyResponseHandler- removed ResponseTransformer conversion (src/app/v1/_lib/proxy/response-handler.ts:~93 lines removed)format-mapper.ts- reduced from ~78 to ~5 effective lines (src/app/v1/_lib/proxy/format-mapper.ts)provider-selector.ts- enforces format compatibility (src/app/v1/_lib/proxy/provider-selector.ts)session-extractor.ts- removed Codex-specific handlingUI Changes
joinClaudePoolfield (legacy pool joining feature)codexInstructionsStrategyfield (Codex instruction injection strategy)Documentation
Tests
tests/unit/proxy/provider-selector-format-compatibility.test.ts(289 lines) - validates format matchingchat-completions-handler-guard-pipeline.test.ts(545 lines)codex-request-sanitizer.test.ts(50 lines)converters-tool-result-nonstream.test.ts(86 lines)openai-to-codex-request.test.ts(50 lines)session-extractor.test.ts- removed Codex-specific test casesBreaking Changes
Cross-format conversion is no longer supported.
Requests must be routed to providers with matching API formats:
Migration Path:
providerTypematches the API format you're sendingRemoved Features:
joinClaudePoolprovider configuration optioncodexInstructionsStrategyprovider configuration optionFiles Changed
Summary: 82 files changed (+357/-9807 lines)
Checklist
Description enhanced by Claude AI