feat: add Claude chat completions proxy mode#171
Open
awsl233777 wants to merge 4 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new ChangesClaude Proxy Dual-Mode Routing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 🔧 ast-grep (0.42.3)web-ui/res/web-ui-render.precompiled.jsThanks 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/unit/web-ui-logic.test.mjs (1)
84-87: ⚡ Quick winCover canonical
chat_completionsin normalization test.This test validates two aliases but not the canonical value itself; adding it will better lock the API contract.
Proposed test addition
test('normalizeClaudeConfig accepts chat completions target api aliases', () => { + assert.strictEqual(normalizeClaudeConfig({ targetApi: 'chat_completions' }).targetApi, 'chat_completions'); assert.strictEqual(normalizeClaudeConfig({ targetApi: 'chat/completions' }).targetApi, 'chat_completions'); assert.strictEqual(normalizeClaudeConfig({ targetApi: 'chat-completions' }).targetApi, 'chat_completions'); });🤖 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/web-ui-logic.test.mjs` around lines 84 - 87, Add an assertion to the existing test that covers the canonical value: call normalizeClaudeConfig with targetApi: 'chat_completions' and assert its targetApi remains 'chat_completions' so the test verifies aliases plus the canonical form; update the test block in tests/unit/web-ui-logic.test.mjs around the normalizeClaudeConfig assertions to include this additional assertion referencing normalizeClaudeConfig and targetApi.tests/unit/claude-settings-sync.test.mjs (1)
480-513: ⚡ Quick winAdd a preservation case for non-default
targetApiduring merge.Current test only validates default
responses; it doesn’t catch regressions where an existingchat_completionsvalue is unintentionally reset on edit.Proposed additional assertion
test('mergeClaudeConfig preserves externalCredentialType across edits without api key', () => { @@ assert.deepStrictEqual(merged, { apiKey: '', baseUrl: 'https://api.anthropic.com/', model: 'claude-3-7-sonnet', hasKey: true, externalCredentialType: 'auth-token', targetApi: 'responses' }); + + const mergedChat = mergeClaudeConfig.call(context, { + apiKey: '', + baseUrl: 'https://api.anthropic.com', + model: 'claude-3-7-sonnet', + hasKey: true, + externalCredentialType: 'auth-token', + targetApi: 'chat_completions' + }, { + model: 'claude-sonnet-4-6' + }); + assert.strictEqual(mergedChat.targetApi, 'chat_completions'); });🤖 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/claude-settings-sync.test.mjs` around lines 480 - 513, The test currently only verifies that mergeClaudeConfig preserves defaults (targetApi='responses') and misses regressions where an existing non-default targetApi (e.g., 'chat_completions') is lost on merge; update the test for mergeClaudeConfig in tests/unit/claude-settings-sync.test.mjs to include an input/initial config with targetApi set to a non-default value (such as 'chat_completions') and assert that the merged result retains that explicit targetApi rather than being reset to 'responses'; use the same context.normalizeClaudeConfig and the mergeClaudeConfig function reference to craft the call and expected assertion so the test will fail if mergeClaudeConfig incorrectly overwrites non-default targetApi.tests/e2e/test-claude-proxy.js (1)
335-342: ⚡ Quick winUse predicate-based request matching instead of positional indexes.
Index-based checks can become flaky if an extra upstream chat call is introduced (retry, preflight, future hook). Matching by payload shape keeps this test stable.
Proposed assertion hardening
const upstreamChatMessages = upstream.requests.filter((item) => item.path === '/v1/chat/completions'); assert(upstreamChatMessages.length >= 2, 'claude proxy should hit upstream /v1/chat/completions'); -assert(upstreamChatMessages[0].headers.authorization === 'Bearer sk-claude-upstream', 'claude proxy chat should use provider auth for upstream'); -assert(upstreamChatMessages[0].body.messages[0].role === 'system', 'claude proxy chat should map system prompt to system message'); -assert(upstreamChatMessages[0].body.max_tokens === 128, 'claude proxy chat should map max_tokens to max_tokens'); -assert(upstreamChatMessages[0].body.stream === false, 'claude proxy chat should synthesize Anthropic streaming locally'); -assert(upstreamChatMessages[1].body.tool_choice.function.name === 'lookup', 'claude proxy chat should map tool_choice'); +const plainChatReq = upstreamChatMessages.find((item) => + item.body && item.body.model === 'DeepSeek-V4-pro' && !item.body.tools +); +const toolChatReq = upstreamChatMessages.find((item) => + item.body && Array.isArray(item.body.tools) && item.body.tools.length > 0 +); +assert(plainChatReq && plainChatReq.headers.authorization === 'Bearer sk-claude-upstream', 'claude proxy chat should use provider auth for upstream'); +assert(plainChatReq.body.messages[0].role === 'system', 'claude proxy chat should map system prompt to system message'); +assert(plainChatReq.body.max_tokens === 128, 'claude proxy chat should map max_tokens to max_tokens'); +assert(plainChatReq.body.stream === false, 'claude proxy chat should synthesize Anthropic streaming locally'); +assert(toolChatReq && toolChatReq.body.tool_choice.function.name === 'lookup', 'claude proxy chat should map tool_choice');🤖 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/e2e/test-claude-proxy.js` around lines 335 - 342, The tests are using positional indexes on upstreamChatMessages which is flaky; update the assertions to locate requests by predicate on their payload shape instead of array position: use upstream.requests.filter(...) or find(...) to match by headers.authorization === 'Bearer sk-claude-upstream' for the provider-auth check, match a request whose body.messages[0].role === 'system' and body.max_tokens === 128 and body.stream === false for the system/max_tokens/stream assertions, and match a request whose body.tool_choice?.function?.name === 'lookup' for the tool_choice assertion; replace the index-based assertions that reference upstreamChatMessages[0] and [1] with these predicate-found entries and assert they exist and have the expected properties.
🤖 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 `@web-ui/partials/index/modals-basic.html`:
- Around line 136-143: Replace the hardcoded Chinese label, option texts and
hint for the target API selector with i18n keys rendered through t(...);
specifically update the strings inside the select block bound to
newClaudeConfig.targetApi (the label "目标 API", the two option texts "Anthropic /
OpenAI Responses" and "OpenAI Chat Completions (/v1/chat/completions)", and the
form-hint copy) to use translation keys (e.g., t('claude.targetApi.label'),
t('claude.targetApi.option.responses'),
t('claude.targetApi.option.chat_completions'), t('claude.targetApi.hint')) and
ensure the same change is applied to the duplicated block around the other
selector (the block you modified at the later occurrence). Ensure keys exist in
the locale files.
In `@web-ui/partials/index/panel-config-claude.html`:
- Line 132: The subtitle text inside the div with class "card-subtitle" (the
element gated by v-if="config.targetApi === 'chat_completions'") is hardcoded;
replace it with a localized string by calling the translation helper (e.g.,
t('panel.mode.openai_chat_completions')) so it follows the app locale, and add
the corresponding translation key to the i18n resources; ensure the
component/context provides the t(...) function before using it.
In `@web-ui/res/web-ui-render.precompiled.js`:
- Around line 1890-1894: The hardcoded UI strings shown when checking
config.targetApi (e.g., the "OpenAI Chat Completions" subtitle rendered in the
_createElementBlock with class "card-subtitle", plus the field label, option
labels and hint around the same conditional branches at the other locations)
must be replaced with i18n keys and rendered via _ctx.t(...). Update the three
affected render sites to call _ctx.t('key_name') instead of raw text, add
appropriate i18n keys for the card subtitle, field label, option labels and hint
(use descriptive keys like "card.subtitle.openai_chat_completions",
"field.label.target_api", "option.label.x", "field.hint.target_api"), and ensure
the conditional branches still pick the right key based on config.targetApi so
translations are used for language switching.
---
Nitpick comments:
In `@tests/e2e/test-claude-proxy.js`:
- Around line 335-342: The tests are using positional indexes on
upstreamChatMessages which is flaky; update the assertions to locate requests by
predicate on their payload shape instead of array position: use
upstream.requests.filter(...) or find(...) to match by headers.authorization ===
'Bearer sk-claude-upstream' for the provider-auth check, match a request whose
body.messages[0].role === 'system' and body.max_tokens === 128 and body.stream
=== false for the system/max_tokens/stream assertions, and match a request whose
body.tool_choice?.function?.name === 'lookup' for the tool_choice assertion;
replace the index-based assertions that reference upstreamChatMessages[0] and
[1] with these predicate-found entries and assert they exist and have the
expected properties.
In `@tests/unit/claude-settings-sync.test.mjs`:
- Around line 480-513: The test currently only verifies that mergeClaudeConfig
preserves defaults (targetApi='responses') and misses regressions where an
existing non-default targetApi (e.g., 'chat_completions') is lost on merge;
update the test for mergeClaudeConfig in
tests/unit/claude-settings-sync.test.mjs to include an input/initial config with
targetApi set to a non-default value (such as 'chat_completions') and assert
that the merged result retains that explicit targetApi rather than being reset
to 'responses'; use the same context.normalizeClaudeConfig and the
mergeClaudeConfig function reference to craft the call and expected assertion so
the test will fail if mergeClaudeConfig incorrectly overwrites non-default
targetApi.
In `@tests/unit/web-ui-logic.test.mjs`:
- Around line 84-87: Add an assertion to the existing test that covers the
canonical value: call normalizeClaudeConfig with targetApi: 'chat_completions'
and assert its targetApi remains 'chat_completions' so the test verifies aliases
plus the canonical form; update the test block in
tests/unit/web-ui-logic.test.mjs around the normalizeClaudeConfig assertions to
include this additional assertion referencing normalizeClaudeConfig and
targetApi.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ae0fbd9-703c-4ef4-b64e-cefdd78b6682
📒 Files selected for processing (13)
cli.jscli/claude-proxy.jstests/e2e/test-claude-proxy.jstests/unit/claude-proxy-adapter.test.mjstests/unit/claude-settings-sync.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/logic.claude.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.startup-claude.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-config-claude.htmlweb-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (10)
cli.js (1)
293-294: LGTM!Also applies to: 5492-5494
cli/claude-proxy.js (2)
136-151: LGTM!Also applies to: 238-352, 432-505, 1268-1270
644-647: LGTM!Also applies to: 676-683, 757-784, 810-813, 1015-1015, 1041-1041, 1074-1077, 1080-1084, 1097-1099, 1198-1199, 1247-1247
web-ui/logic.claude.mjs (1)
72-75: LGTM!Also applies to: 82-83
web-ui/app.js (1)
278-279: LGTM!Also applies to: 284-285, 292-294, 539-542
web-ui/modules/app.methods.claude-config.mjs (1)
55-57: LGTM!Also applies to: 67-69, 88-88, 110-110, 186-186, 208-210
web-ui/modules/app.methods.startup-claude.mjs (1)
250-252: LGTM!tests/unit/claude-proxy-adapter.test.mjs (1)
7-10: LGTM!Also applies to: 67-105, 145-172
tests/unit/web-ui-logic.test.mjs (1)
54-55: LGTM!Also applies to: 66-80
tests/e2e/test-claude-proxy.js (1)
101-142: LGTM!Also applies to: 260-334, 343-394
Collaborator
Author
|
Updated the PR to close the Claude Chat Completions apply-path gap and the UI i18n review comment. Changes in this push:
Validation run locally:
@coderabbitai re-review |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
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)
cli.js (1)
9176-9214:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRollback proxy runtime on post-start failure paths.
If the proxy starts successfully but a later step fails (e.g., reading/writing Claude settings), the function returns an error while leaving the proxy running. That creates inconsistent state (
applyfailed, runtime still active).Proposed fix
async function applyToClaudeSettings(config = {}) { + let proxyStarted = false; try { @@ if (targetApi === 'chat_completions') { await stopBuiltinClaudeProxyRuntime(); proxyResult = await startBuiltinClaudeProxyRuntime({ @@ if (!proxyResult || proxyResult.error || proxyResult.success === false || !proxyResult.listenUrl) { + await stopBuiltinClaudeProxyRuntime(); return { success: false, mode: 'claude-proxy', error: (proxyResult && proxyResult.error) || '启动 Claude 兼容代理失败' }; } + proxyStarted = true; settingsBaseUrl = proxyResult.listenUrl; settingsApiKey = 'codexmate'; } else { @@ const readResult = readJsonObjectFromFile(CLAUDE_SETTINGS_FILE, {}); if (!readResult.ok) { + if (proxyStarted) { + await stopBuiltinClaudeProxyRuntime(); + } return { success: false, mode: 'settings-file', error: readResult.error }; } @@ } catch (e) { + if (proxyStarted) { + try { await stopBuiltinClaudeProxyRuntime(); } catch (_) { } + } return { success: false, mode: 'settings-file', error: e.message || '应用 Claude 配置失败' }; } }Also applies to: 9262-9267
🤖 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 `@cli.js` around lines 9176 - 9214, The code may leave the built-in Claude proxy running if later steps fail; ensure we stop the proxy on all post-start failure paths by calling stopBuiltinClaudeProxyRuntime() before any early return after a successful start (i.e., when proxyResult indicates success and you later hit an error while handling CLAUDE_SETTINGS_FILE, readJsonObjectFromFile, or writeJsonAtomic). Specifically, after using startBuiltinClaudeProxyRuntime() and assigning settingsBaseUrl/settingsApiKey (and likewise in the branch that writes BUILTIN_CLAUDE_PROXY_SETTINGS_FILE), add logic to call stopBuiltinClaudeProxyRuntime() (or otherwise tear down the runtime) whenever you are about to return an error, so proxyResult/started runtimes are cleaned up consistently.
🤖 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 `@cli.js`:
- Around line 9178-9197: The code currently sets a predictable proxy credential
(settingsApiKey = 'codexmate') after calling startBuiltinClaudeProxyRuntime,
which makes the proxy guessable if bound to a non-loopback address; replace this
by generating a cryptographically-secure random token for settingsApiKey (e.g.,
using crypto.randomBytes) and assign that token instead of the constant, and
additionally validate proxyResult.listenUrl (from
startBuiltinClaudeProxyRuntime) to ensure it is bound to a loopback address
(127.0.0.1/::1); if the listenUrl is non-loopback either refuse to use a
fixed/predictable key (fail or require explicit config) or log and rotate a
random token, ensuring settingsBaseUrl uses proxyResult.listenUrl and only the
random token is exposed as the API key.
---
Outside diff comments:
In `@cli.js`:
- Around line 9176-9214: The code may leave the built-in Claude proxy running if
later steps fail; ensure we stop the proxy on all post-start failure paths by
calling stopBuiltinClaudeProxyRuntime() before any early return after a
successful start (i.e., when proxyResult indicates success and you later hit an
error while handling CLAUDE_SETTINGS_FILE, readJsonObjectFromFile, or
writeJsonAtomic). Specifically, after using startBuiltinClaudeProxyRuntime() and
assigning settingsBaseUrl/settingsApiKey (and likewise in the branch that writes
BUILTIN_CLAUDE_PROXY_SETTINGS_FILE), add logic to call
stopBuiltinClaudeProxyRuntime() (or otherwise tear down the runtime) whenever
you are about to return an error, so proxyResult/started runtimes are cleaned up
consistently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1c4589c-b7ff-4435-a393-681c33a08def
📒 Files selected for processing (9)
cli.jscli/claude-proxy.jstests/e2e/test-claude.jstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-config-claude.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (1)
- web-ui/modules/i18n.dict.mjs
📜 Review details
🔇 Additional comments (22)
tests/e2e/test-claude.js (1)
67-91: LGTM!web-ui/partials/index/panel-config-claude.html (1)
132-132: Ensureclaude.targetApi.chatCompletionsBadgeis present in i18n resources.
claude.targetApi.chatCompletionsBadgeis defined inweb-ui/modules/i18n.dict.mjsfor all supported locale blocks (zh,ja,en), so thet('claude.targetApi.chatCompletionsBadge')call won’t fail due to a missing key.web-ui/res/web-ui-render.precompiled.js (1)
1894-1894: LGTM!Also applies to: 5561-5571, 5675-5685
tests/unit/web-ui-logic.test.mjs (1)
84-88: LGTM!web-ui/partials/index/modals-basic.html (1)
136-143: LGTM!Also applies to: 175-182
cli.js (1)
9154-9159: LGTM!Also applies to: 9399-9403, 11008-11008, 15556-15558
cli/claude-proxy.js (10)
136-151: LGTM!
238-293: LGTM!
295-352: LGTM!
432-505: LGTM!
816-840: LGTM!
1212-1212: LGTM!
1099-1111: LGTM!
526-617: LGTM!
681-684: LGTM!
757-785: LGTM!web-ui/modules/app.methods.claude-config.mjs (6)
50-59: LGTM!
61-72: LGTM!
85-89: LGTM!
110-127: LGTM!
186-199: LGTM!
202-211: LGTM!
Contributor
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)
cli.js (1)
9176-9267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop the proxy again if the later settings write path fails.
After Line 9178 starts the runtime, any later failure in
readJsonObjectFromFile,backupFileIfNeededOnce, orwriteJsonAtomicreturns an error but never tears the proxy back down. That leaves a leaked background process and persisted proxy state that no longer matches~/.claude/settings.json.🧯 Suggested rollback
async function applyToClaudeSettings(config = {}) { try { const apiKey = (config.apiKey || '').trim(); if (!apiKey) { return { success: false, mode: 'settings-file', error: '请先输入 API Key' }; @@ const targetApi = normalizeClaudeTargetApi(config.targetApi); let settingsBaseUrl = baseUrl; let settingsApiKey = apiKey; let proxyResult = null; + let proxyStarted = false; @@ proxyResult = await startBuiltinClaudeProxyRuntime({ enabled: true, provider: typeof config.name === 'string' ? config.name.trim() : '', authSource: 'provider', targetApi, @@ if (!proxyResult || proxyResult.error || proxyResult.success === false || !proxyResult.listenUrl) { return { success: false, mode: 'claude-proxy', error: (proxyResult && proxyResult.error) || '启动 Claude 兼容代理失败' }; } + proxyStarted = true; settingsBaseUrl = proxyResult.listenUrl; settingsApiKey = 'codexmate'; @@ } catch (e) { + if (proxyStarted) { + try { + await stopBuiltinClaudeProxyRuntime(); + } catch (_) { } + } return { success: false, mode: 'settings-file', error: e.message || '应用 Claude 配置失败' };🤖 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 `@cli.js` around lines 9176 - 9267, The code can leak a running proxy if startBuiltinClaudeProxyRuntime succeeds but later steps (readJsonObjectFromFile, backupFileIfNeededOnce, writeJsonAtomic) fail; modify the flow around startBuiltinClaudeProxyRuntime/stopBuiltinClaudeProxyRuntime so that when proxyResult indicates a proxy was started you always stop it on any early return or exception—either wrap the post-proxy logic in a try/finally that calls stopBuiltinClaudeProxyRuntime() in the finally, or track a boolean like proxyStarted and call stopBuiltinClaudeProxyRuntime() before each error return and in the catch block; ensure this covers the branches that return after readJsonObjectFromFile errors and after writeJsonAtomic/backup failures, and still preserve returning proxy info only when the operation completes successfully.
♻️ Duplicate comments (1)
cli.js (1)
9178-9196:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForce loopback before writing the fixed proxy token.
startBuiltinClaudeProxyRuntime(...)is called without a host override, so a previously persisted non-loopback bind can still be reused here. Line 9196 then writes a constantANTHROPIC_API_KEY, which makes the proxy credential guessable off-box if this runtime comes up anywhere other than loopback.🔒 Minimal hardening
proxyResult = await startBuiltinClaudeProxyRuntime({ enabled: true, + host: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host, provider: typeof config.name === 'string' ? config.name.trim() : '', authSource: 'provider', targetApi, timeoutMs: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.timeoutMs, upstreamProviderName: typeof config.name === 'string' ? config.name.trim() : '', upstreamBaseUrl: baseUrl, upstreamApiKey: apiKey }); if (!proxyResult || proxyResult.error || proxyResult.success === false || !proxyResult.listenUrl) { return { success: false, mode: 'claude-proxy', error: (proxyResult && proxyResult.error) || '启动 Claude 兼容代理失败' }; } + { + const listenHost = new URL(proxyResult.listenUrl).hostname; + if (listenHost !== '127.0.0.1' && listenHost !== 'localhost' && listenHost !== '::1') { + await stopBuiltinClaudeProxyRuntime(); + return { + success: false, + mode: 'claude-proxy', + error: 'Claude 兼容代理在 chat_completions 模式下必须仅监听 loopback' + }; + } + } settingsBaseUrl = proxyResult.listenUrl; settingsApiKey = 'codexmate';🤖 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 `@cli.js` around lines 9178 - 9196, The code currently starts the Claude proxy without forcing a loopback bind and then writes a fixed proxy API key ('codexmate'/ANTHROPIC_API_KEY), which can leak if the runtime binds non-loopback; update the call to startBuiltinClaudeProxyRuntime to force a loopback host (e.g., pass bindHost: '127.0.0.1' or equivalent) and only persist or expose a generated/ephemeral settingsApiKey when proxyResult.listenUrl is loopback; replace the constant 'codexmate' assignment to settingsApiKey with a securely generated random token (or skip persisting the token) when proxyResult.listenUrl is not loopback so the credential cannot be guessed off-box (references: startBuiltinClaudeProxyRuntime, settingsApiKey, ANTHROPIC_API_KEY).
🤖 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 `@cli/claude-proxy.js`:
- Around line 769-776: resolveOpenaiBridgeUpstream may return null/undefined
causing a TypeError when accessing bridgeUpstream.baseUrl; update the logic
around resolveOpenaiBridgeUpstream (the bridgeUpstream variable) to explicitly
check for null/undefined and return an error if so, then only access
bridgeUpstream.baseUrl (used to set bridgeBaseUrl) after confirming
bridgeUpstream is an object and has a baseUrl property; keep existing
isValidHttpUrl(providerName) check and error messages using providerName
unchanged.
---
Outside diff comments:
In `@cli.js`:
- Around line 9176-9267: The code can leak a running proxy if
startBuiltinClaudeProxyRuntime succeeds but later steps (readJsonObjectFromFile,
backupFileIfNeededOnce, writeJsonAtomic) fail; modify the flow around
startBuiltinClaudeProxyRuntime/stopBuiltinClaudeProxyRuntime so that when
proxyResult indicates a proxy was started you always stop it on any early return
or exception—either wrap the post-proxy logic in a try/finally that calls
stopBuiltinClaudeProxyRuntime() in the finally, or track a boolean like
proxyStarted and call stopBuiltinClaudeProxyRuntime() before each error return
and in the catch block; ensure this covers the branches that return after
readJsonObjectFromFile errors and after writeJsonAtomic/backup failures, and
still preserve returning proxy info only when the operation completes
successfully.
---
Duplicate comments:
In `@cli.js`:
- Around line 9178-9196: The code currently starts the Claude proxy without
forcing a loopback bind and then writes a fixed proxy API key
('codexmate'/ANTHROPIC_API_KEY), which can leak if the runtime binds
non-loopback; update the call to startBuiltinClaudeProxyRuntime to force a
loopback host (e.g., pass bindHost: '127.0.0.1' or equivalent) and only persist
or expose a generated/ephemeral settingsApiKey when proxyResult.listenUrl is
loopback; replace the constant 'codexmate' assignment to settingsApiKey with a
securely generated random token (or skip persisting the token) when
proxyResult.listenUrl is not loopback so the credential cannot be guessed
off-box (references: startBuiltinClaudeProxyRuntime, settingsApiKey,
ANTHROPIC_API_KEY).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f3075cd-6ca8-427d-85f1-4f2a07df91b4
📒 Files selected for processing (15)
cli.jscli/claude-proxy.jstests/e2e/test-claude-proxy.jstests/e2e/test-claude.jstests/unit/claude-proxy-adapter.test.mjstests/unit/claude-settings-sync.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/logic.claude.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.startup-claude.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-config-claude.htmlweb-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (22)
web-ui/partials/index/panel-config-claude.html (1)
132-132: LGTM!web-ui/logic.claude.mjs (1)
72-75: LGTM!Also applies to: 82-83
web-ui/app.js (1)
278-278: LGTM!Also applies to: 284-284, 292-294, 539-542
web-ui/partials/index/modals-basic.html (1)
136-143: LGTM!Also applies to: 175-182
web-ui/modules/i18n.dict.mjs (1)
1051-1055: LGTM!Also applies to: 2112-2116, 3183-3187
tests/unit/claude-settings-sync.test.mjs (1)
490-492: LGTM!Also applies to: 511-513
tests/unit/claude-proxy-adapter.test.mjs (1)
7-7: LGTM!Also applies to: 9-9, 67-105, 145-172
tests/unit/web-ui-logic.test.mjs (1)
54-55: LGTM!Also applies to: 67-68, 79-80, 84-88
tests/e2e/test-claude.js (1)
67-84: LGTM!Also applies to: 90-92
tests/e2e/test-claude-proxy.js (1)
101-142: LGTM!Also applies to: 260-384, 392-394
web-ui/modules/app.methods.startup-claude.mjs (1)
250-252: LGTM!web-ui/res/web-ui-render.precompiled.js (1)
1890-1900: LGTM!Also applies to: 5560-5572, 5674-5686
web-ui/modules/app.methods.claude-config.mjs (1)
55-56: LGTM!Also applies to: 67-68, 88-88, 110-110, 112-112, 186-186, 188-188, 208-209
cli.js (1)
293-294: LGTM!Also applies to: 5492-5494, 9154-9159, 9399-9403, 11007-11008, 15556-15564
cli/claude-proxy.js (8)
136-151: LGTM!
238-293: LGTM!
295-352: LGTM!
432-505: LGTM!
644-695: LGTM!
816-839: LGTM!
1100-1125: LGTM!
1224-1224: LGTM!Also applies to: 1273-1273, 1294-1296
Collaborator
Author
|
Pushed a follow-up hardening commit for the latest CodeRabbit actionable comments: Changes:
Validation run locally after this commit:
@coderabbitai re-review |
Contributor
|
Triggering a full re-review now. 🐇✨ ✅ Actions performedFull review triggered. |
Collaborator
Author
|
Systematic test pass added for the Claude What changed in this push:
Validation run locally:
Additional real-upstream smoke:
@coderabbitai re-review |
Contributor
|
Triggering a full re-review now. 🐇✨ ✅ Actions performedFull review triggered. |
Contributor
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 (1)
cli.js (1)
9216-9251:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDelay proxy shutdown until the settings write succeeds.
In the
responsesbranch, Lines 9217-9218 tear down the existing proxy beforeCLAUDE_SETTINGS_FILEis even read. If the later read/write fails, apply returns an error but the previously working proxy is already gone.Suggested fix
if (targetApi === 'chat_completions') { await stopBuiltinClaudeProxyRuntime(); const proxyToken = crypto.randomBytes(24).toString('hex'); proxyResult = await startBuiltinClaudeProxyRuntime({ enabled: true, host: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host, provider: typeof config.name === 'string' ? config.name.trim() : '', authSource: 'provider', targetApi, timeoutMs: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.timeoutMs, upstreamProviderName: typeof config.name === 'string' ? config.name.trim() : '', upstreamBaseUrl: baseUrl, upstreamApiKey: apiKey }); if (!proxyResult || proxyResult.error || proxyResult.success === false || !proxyResult.listenUrl) { await stopBuiltinClaudeProxyRuntime(); resetBuiltinClaudeProxySavedSettingsToResponses(); return { success: false, mode: 'claude-proxy', error: (proxyResult && proxyResult.error) || '启动 Claude 兼容代理失败' }; } proxyStarted = true; settingsBaseUrl = proxyResult.listenUrl; settingsApiKey = proxyToken; - } else { - await stopBuiltinClaudeProxyRuntime(); - resetBuiltinClaudeProxySavedSettingsToResponses(); } @@ ensureDir(CLAUDE_DIR); const backupPath = backupFileIfNeededOnce(CLAUDE_SETTINGS_FILE); writeJsonAtomic(CLAUDE_SETTINGS_FILE, nextSettings); + if (targetApi !== 'chat_completions') { + await stopBuiltinClaudeProxyRuntime(); + resetBuiltinClaudeProxySavedSettingsToResponses(); + }
🤖 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 `@tests/e2e/test-claude-proxy.js`:
- Around line 356-370: Wrap the block that mutates and tests the bridge settings
in a try...finally so the original file is always restored: move the
fs.writeFileSync(bridgeSettingsPath, JSON.stringify({ providers: {} }, ...)) and
the subsequent api('claude-proxy-start') / asserts into a try block and put
fs.writeFileSync(bridgeSettingsPath, savedBridgeSettings, 'utf-8') inside a
finally block; reference the bridgeSettingsPath and savedBridgeSettings
variables and ensure the api calls missingBridgeStartResult and
missingBridgeStatus remain inside the try so any thrown error still triggers the
finally restoration.
In `@tests/e2e/test-claude.js`:
- Around line 99-109: The test corrupts the Claude settings.json
(claudeSettingsPath) but only restores it at the end, so failures leave the file
corrupted; wrap the mutation and subsequent assertions (the call to
api('apply-claude-config') and the checks against failedChatApply and
claudeProxyStatusAfterFailedApply from api('claude-proxy-status')) in a
try/finally and always write back validClaudeSettings to claudeSettingsPath in
the finally block to guarantee restoration even if an assertion throws.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5ad0bbee-40c7-483e-83ae-36c3bd62ac49
📒 Files selected for processing (4)
cli.jscli/claude-proxy.jstests/e2e/test-claude-proxy.jstests/e2e/test-claude.js
📜 Review details
🔇 Additional comments (3)
cli.js (2)
9161-9172: LGTM!
9192-9215: ⚡ Quick winRemove the “keep proxyToken and ANTHROPIC_API_KEY in sync” requirement
proxyTokenis only generated incli.jsand written toenv.ANTHROPIC_API_KEY, butstartBuiltinClaudeProxyRuntime(...)is called without it. The proxy’s incoming authentication is driven byprocess.env.CODEXMATE_HTTP_TOKENand checksAuthorization/x-codexmate-tokenheaders (notx-api-key/ANTHROPIC_API_KEY), so Claude’sANTHROPIC_API_KEYvalue doesn’t need to matchproxyTokenfor the proxy to accept requests.> Likely an incorrect or invalid review comment.cli/claude-proxy.js (1)
769-771: LGTM!
Comment on lines
+99
to
+109
| const claudeSettingsPath = path.join(tmpHome, '.claude', 'settings.json'); | ||
| const validClaudeSettings = fs.readFileSync(claudeSettingsPath, 'utf-8'); | ||
| fs.writeFileSync(claudeSettingsPath, '{ invalid json', 'utf-8'); | ||
| const failedChatApply = await api('apply-claude-config', { | ||
| config: { name: 'claude-chat-direct', baseUrl: mockProviderUrl, apiKey: 'sk-new', model: 'new-model', targetApi: 'chat_completions' } | ||
| }); | ||
| assert(failedChatApply.success === false || failedChatApply.error, 'apply-claude-config should fail when Claude settings cannot be read'); | ||
| const claudeProxyStatusAfterFailedApply = await api('claude-proxy-status'); | ||
| assert(claudeProxyStatusAfterFailedApply.running === false, 'failed chat_completions apply should roll back the Claude proxy runtime'); | ||
| assert(claudeProxyStatusAfterFailedApply.settings && claudeProxyStatusAfterFailedApply.settings.targetApi === 'responses', 'failed chat_completions apply should reset saved Claude proxy targetApi'); | ||
| fs.writeFileSync(claudeSettingsPath, validClaudeSettings, 'utf-8'); |
Contributor
There was a problem hiding this comment.
Always restore the Claude settings file in this rollback test.
If any call/assertion in Lines 102-108 fails, settings.json stays corrupted and can poison later E2E checks. Please wrap the temporary file mutation in a try/finally.
Suggested fix
const claudeSettingsPath = path.join(tmpHome, '.claude', 'settings.json');
const validClaudeSettings = fs.readFileSync(claudeSettingsPath, 'utf-8');
-fs.writeFileSync(claudeSettingsPath, '{ invalid json', 'utf-8');
-const failedChatApply = await api('apply-claude-config', {
- config: { name: 'claude-chat-direct', baseUrl: mockProviderUrl, apiKey: 'sk-new', model: 'new-model', targetApi: 'chat_completions' }
-});
-assert(failedChatApply.success === false || failedChatApply.error, 'apply-claude-config should fail when Claude settings cannot be read');
-const claudeProxyStatusAfterFailedApply = await api('claude-proxy-status');
-assert(claudeProxyStatusAfterFailedApply.running === false, 'failed chat_completions apply should roll back the Claude proxy runtime');
-assert(claudeProxyStatusAfterFailedApply.settings && claudeProxyStatusAfterFailedApply.settings.targetApi === 'responses', 'failed chat_completions apply should reset saved Claude proxy targetApi');
-fs.writeFileSync(claudeSettingsPath, validClaudeSettings, 'utf-8');
+try {
+ fs.writeFileSync(claudeSettingsPath, '{ invalid json', 'utf-8');
+ const failedChatApply = await api('apply-claude-config', {
+ config: { name: 'claude-chat-direct', baseUrl: mockProviderUrl, apiKey: 'sk-new', model: 'new-model', targetApi: 'chat_completions' }
+ });
+ assert(failedChatApply.success === false || failedChatApply.error, 'apply-claude-config should fail when Claude settings cannot be read');
+ const claudeProxyStatusAfterFailedApply = await api('claude-proxy-status');
+ assert(claudeProxyStatusAfterFailedApply.running === false, 'failed chat_completions apply should roll back the Claude proxy runtime');
+ assert(claudeProxyStatusAfterFailedApply.settings && claudeProxyStatusAfterFailedApply.settings.targetApi === 'responses', 'failed chat_completions apply should reset saved Claude proxy targetApi');
+} finally {
+ fs.writeFileSync(claudeSettingsPath, validClaudeSettings, 'utf-8');
+}🤖 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/e2e/test-claude.js` around lines 99 - 109, The test corrupts the Claude
settings.json (claudeSettingsPath) but only restores it at the end, so failures
leave the file corrupted; wrap the mutation and subsequent assertions (the call
to api('apply-claude-config') and the checks against failedChatApply and
claudeProxyStatusAfterFailedApply from api('claude-proxy-status')) in a
try/finally and always write back validClaudeSettings to claudeSettingsPath in
the finally block to guarantee restoration even if an assertion throws.
Contributor
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 (1)
web-ui/app.js (1)
534-543:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard malformed persisted Claude config entries before assigning
targetApi.On Line 540,
config.targetApi = ...assumesconfigis an object. Corrupted localStorage entries can throw here and break startup normalization.Suggested fix
for (const [name, config] of Object.entries(this.claudeConfigs)) { + if (!config || typeof config !== 'object' || Array.isArray(config)) { + delete this.claudeConfigs[name]; + continue; + } if (config.apiKey && config.apiKey.includes('****')) { config.apiKey = ''; config.hasKey = false; } const targetApiRaw = typeof config.targetApi === 'string' ? config.targetApi.trim().toLowerCase() : ''; config.targetApi = targetApiRaw === 'chat_completions' || targetApiRaw === 'chat-completions' || targetApiRaw === 'chat/completions' ? 'chat_completions' : 'responses'; }🤖 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 `@web-ui/app.js` around lines 534 - 543, The loop over this.claudeConfigs assumes each entry is a valid object; guard against malformed persisted entries by checking that config is a non-null object before reading or writing properties like config.apiKey, config.hasKey and config.targetApi, and if an entry is invalid either skip normalization or replace it with a safe default object (e.g., { apiKey: '', hasKey: false, targetApi: 'responses' }) so the subsequent assignment to config.targetApi cannot throw.
🤖 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 `@cli.js`:
- Around line 9183-9203: The code allows targetApi === 'chat_completions' to
startBuiltinClaudeProxyRuntime even when config.baseUrl is empty (falling back
to the Anthropic Messages URL), which silently wires the proxy to the wrong
upstream; update the validation around normalizeClaudeTargetApi/targetApi and
baseUrl before calling startBuiltinClaudeProxyRuntime so that if targetApi ===
'chat_completions' and config.baseUrl is missing or equal to the default
Anthropic messages URL you either throw/return a validation error or call
startBuiltinClaudeProxyRuntime without upstreamBaseUrl (omit the upstreamBaseUrl
property) so the runtime resolves the upstream from config.name; adjust the
block that prepares proxyResult (the call to startBuiltinClaudeProxyRuntime and
the upstreamBaseUrl/upstreamProviderName fields) accordingly to enforce this
check.
- Around line 9192-9203: The generated proxyToken from
applyToClaudeSettings(...) is not passed into startBuiltinClaudeProxyRuntime,
causing an auth mismatch; update the call to startBuiltinClaudeProxyRuntime (the
invocation that currently sets host, provider, authSource, targetApi, timeoutMs,
upstreamProviderName, upstreamBaseUrl, upstreamApiKey) to include the generated
proxyToken (e.g., as an authToken or proxyToken parameter) so the runtime
enforces the same token that was written into Claude’s ANTHROPIC_API_KEY, and
also verify cli/claude-proxy.js checks that token (or falls back to
CODEXMATE_HTTP_TOKEN only when appropriate) for non-loopback requests when
DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host is non-loopback. Ensure parameter
naming matches startBuiltinClaudeProxyRuntime’s signature and update any
doc/comment accordingly.
---
Outside diff comments:
In `@web-ui/app.js`:
- Around line 534-543: The loop over this.claudeConfigs assumes each entry is a
valid object; guard against malformed persisted entries by checking that config
is a non-null object before reading or writing properties like config.apiKey,
config.hasKey and config.targetApi, and if an entry is invalid either skip
normalization or replace it with a safe default object (e.g., { apiKey: '',
hasKey: false, targetApi: 'responses' }) so the subsequent assignment to
config.targetApi cannot throw.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 858ec6da-e7ea-42fc-b114-09db6512dd0d
📒 Files selected for processing (15)
cli.jscli/claude-proxy.jstests/e2e/test-claude-proxy.jstests/e2e/test-claude.jstests/unit/claude-proxy-adapter.test.mjstests/unit/claude-settings-sync.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/logic.claude.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.startup-claude.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-config-claude.htmlweb-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (9)
cli.js (1)
293-294: LGTM!Also applies to: 5492-5494, 9154-9172, 9417-9421, 11025-11026
tests/e2e/test-claude-proxy.js (1)
356-370: Always restore bridge settings withtry/finally.Line 358 mutates
codexmate-openai-bridge.json, but restore on Line 370 is not guaranteed if an assertion or API call fails first.tests/e2e/test-claude.js (1)
99-109: Wrap Claude settings corruption/restore intry/finally.Line 101 corrupts
settings.json, but restoration on Line 109 is skipped if intermediate assertions fail.web-ui/modules/app.methods.claude-config.mjs (1)
55-56: LGTM!Also applies to: 67-68, 88-88, 110-113, 186-189, 208-209
web-ui/modules/app.methods.startup-claude.mjs (1)
250-251: LGTM!web-ui/modules/i18n.dict.mjs (1)
1051-1055: LGTM!Also applies to: 2112-2116, 3183-3187
web-ui/partials/index/modals-basic.html (1)
136-143: LGTM!Also applies to: 175-182
web-ui/partials/index/panel-config-claude.html (1)
132-132: LGTM!web-ui/res/web-ui-render.precompiled.js (1)
1890-1900: LGTM!Also applies to: 5560-5572, 5674-5686
Comment on lines
9183
to
+9203
| const baseUrl = (config.baseUrl || 'https://open.bigmodel.cn/api/anthropic').trim(); | ||
| const model = (config.model || DEFAULT_CLAUDE_MODEL).trim(); | ||
| const targetApi = normalizeClaudeTargetApi(config.targetApi); | ||
| let settingsBaseUrl = baseUrl; | ||
| let settingsApiKey = apiKey; | ||
| let proxyResult = null; | ||
|
|
||
| if (targetApi === 'chat_completions') { | ||
| await stopBuiltinClaudeProxyRuntime(); | ||
| const proxyToken = crypto.randomBytes(24).toString('hex'); | ||
| proxyResult = await startBuiltinClaudeProxyRuntime({ | ||
| enabled: true, | ||
| host: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host, | ||
| provider: typeof config.name === 'string' ? config.name.trim() : '', | ||
| authSource: 'provider', | ||
| targetApi, | ||
| timeoutMs: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.timeoutMs, | ||
| upstreamProviderName: typeof config.name === 'string' ? config.name.trim() : '', | ||
| upstreamBaseUrl: baseUrl, | ||
| upstreamApiKey: apiKey | ||
| }); |
Contributor
There was a problem hiding this comment.
Reject chat_completions mode without an explicit upstream URL.
When config.baseUrl is empty, this path still falls back to the Anthropic Messages base URL and passes it into upstreamBaseUrl. That silently creates a chat_completions proxy against the wrong upstream instead of failing fast. Return a validation error here, or omit upstreamBaseUrl so the runtime can resolve it from config.name if that's supported.
Possible fix
- const baseUrl = (config.baseUrl || 'https://open.bigmodel.cn/api/anthropic').trim();
+ const configuredBaseUrl = typeof config.baseUrl === 'string' ? config.baseUrl.trim() : '';
+ const baseUrl = (configuredBaseUrl || 'https://open.bigmodel.cn/api/anthropic').trim();
const model = (config.model || DEFAULT_CLAUDE_MODEL).trim();
const targetApi = normalizeClaudeTargetApi(config.targetApi);
@@
if (targetApi === 'chat_completions') {
+ if (!configuredBaseUrl) {
+ return { success: false, mode: 'claude-proxy', error: 'chat_completions 模式需要显式的上游 Base URL' };
+ }
await stopBuiltinClaudeProxyRuntime();
const proxyToken = crypto.randomBytes(24).toString('hex');
proxyResult = await startBuiltinClaudeProxyRuntime({
@@
- upstreamBaseUrl: baseUrl,
+ upstreamBaseUrl: configuredBaseUrl,
upstreamApiKey: apiKey
});🤖 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 `@cli.js` around lines 9183 - 9203, The code allows targetApi ===
'chat_completions' to startBuiltinClaudeProxyRuntime even when config.baseUrl is
empty (falling back to the Anthropic Messages URL), which silently wires the
proxy to the wrong upstream; update the validation around
normalizeClaudeTargetApi/targetApi and baseUrl before calling
startBuiltinClaudeProxyRuntime so that if targetApi === 'chat_completions' and
config.baseUrl is missing or equal to the default Anthropic messages URL you
either throw/return a validation error or call startBuiltinClaudeProxyRuntime
without upstreamBaseUrl (omit the upstreamBaseUrl property) so the runtime
resolves the upstream from config.name; adjust the block that prepares
proxyResult (the call to startBuiltinClaudeProxyRuntime and the
upstreamBaseUrl/upstreamProviderName fields) accordingly to enforce this check.
Comment on lines
+9192
to
+9203
| const proxyToken = crypto.randomBytes(24).toString('hex'); | ||
| proxyResult = await startBuiltinClaudeProxyRuntime({ | ||
| enabled: true, | ||
| host: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host, | ||
| provider: typeof config.name === 'string' ? config.name.trim() : '', | ||
| authSource: 'provider', | ||
| targetApi, | ||
| timeoutMs: DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.timeoutMs, | ||
| upstreamProviderName: typeof config.name === 'string' ? config.name.trim() : '', | ||
| upstreamBaseUrl: baseUrl, | ||
| upstreamApiKey: apiKey | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli.js callsite =="
rg -n -C3 'startBuiltinClaudeProxyRuntime\(' cli.js
echo
echo "== Claude proxy runtime/controller auth handling =="
rg -n -C4 'createBuiltinClaudeProxyRuntimeController|startBuiltinClaudeProxyRuntime|expected(ApiKey|Token)|proxyToken|authorization|x-api-key|ANTHROPIC_API_KEY|auth' cli/claude-proxy.jsRepository: SakuraByteCore/codexmate
Length of output: 11163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for CLAUDE_SETTINGS_FILE and proxyToken/expected token fields =="
rg -n "CLAUDE_SETTINGS_FILE|proxyToken|expectedApiKey|expected.*token|x-api-key|settings\.json|authHeader|authSource" cli/claude-proxy.js
echo
echo "== Inspect startBuiltinClaudeProxyRuntime implementation (payload merging / settings defaults) =="
rg -n "async function startBuiltinClaudeProxyRuntime|function readBuiltinClaudeProxySettings|saveBuiltinClaudeProxySettings" cli/claude-proxy.js
python3 - <<'PY'
import re,sys,subprocess,os, textwrap, json, pathlib
path="cli/claude-proxy.js"
# Print a focused window around startBuiltinClaudeProxyRuntime if found
import subprocess
out=subprocess.check_output(["rg","-n","async function startBuiltinClaudeProxyRuntime","cli/claude-proxy.js"],text=True)
m=re.search(r":(\d+):",out)
start=int(m.group(1))
print(f"start line: {start}")
# Print range start-40..start+80
import itertools
with open(path,"r",encoding="utf-8",errors="replace") as f:
lines=f.readlines()
lo=max(0,start-40)
hi=min(len(lines),start+120)
for i in range(lo,hi):
print(f"{i+1}:{lines[i].rstrip()}")
PY
echo
echo "== Inspect the code that handles auth for built-in proxy requests (Claude -> proxy) =="
python3 - <<'PY'
import re,subprocess
path="cli/claude-proxy.js"
# Find where request handler calls buildBuiltinClaudeProxyRequestAuthHeader and where headers are checked
patterns=["buildBuiltinClaudeProxyRequestAuthHeader","writeAnthropicProxyError","handleBuiltinClaudeProxyRequest","Unauthorized","CODEXMATE_HTTP_TOKEN"]
for pat in patterns:
try:
out=subprocess.check_output(["rg","-n",pat,path],text=True)
except subprocess.CalledProcessError:
continue
print(f"\n-- matches for {pat} --")
print(out)
PY
echo
echo "== Inspect around buildBuiltinClaudeProxyRequestAuthHeader + incoming request checks =="
python3 - <<'PY'
import re
path="cli/claude-proxy.js"
# Locate buildBuiltinClaudeProxyRequestAuthHeader definition line
import subprocess
out=subprocess.check_output(["rg","-n","function buildBuiltinClaudeProxyRequestAuthHeader","cli/claude-proxy.js"],text=True)
m=re.search(r":(\d+):",out)
start=int(m.group(1))
with open(path,'r',encoding='utf-8',errors='replace') as f:
lines=f.readlines()
lo=max(0,start-30); hi=min(len(lines),start+140)
for i in range(lo,hi):
print(f"{i+1}:{lines[i].rstrip()}")
PY
echo
echo "== Inspect how upstream auth is derived from settings (provider/authSource/profile/none) =="
python3 - <<'PY'
import re,subprocess
path="cli/claude-proxy.js"
# Locate resolveBuiltinClaudeProxyUpstream start
out=subprocess.check_output(["rg","-n","function resolveBuiltinClaudeProxyUpstream|resolveBuiltinClaudeProxyUpstream","cli/claude-proxy.js"],text=True)
# choose line with function resolveBuiltinClaudeProxyUpstream(deps?) etc.
# just print first match line number
import itertools
lines=out.strip().splitlines()
first=lines[0]
m=re.search(r":(\d+):",first)
start=int(m.group(1))
print(f"first match line: {start} ({first})")
with open(path,'r',encoding='utf-8',errors='replace') as f:
arr=f.readlines()
lo=max(0,start-20); hi=min(len(arr),start+220)
for i in range(lo,hi):
print(f"{i+1}:{arr[i].rstrip()}")
PYRepository: SakuraByteCore/codexmate
Length of output: 2202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Show read/save settings helpers in cli/claude-proxy.js (around 680-760) =="
nl -ba cli/claude-proxy.js | sed -n '660,780p'
echo
echo "== Show startBuiltinClaudeProxyRuntime implementation (around 1180-1260) =="
nl -ba cli/claude-proxy.js | sed -n '1170,1265p'
echo
echo "== Find/inspect resolveAuthTokenFromCurrentProfile wiring (usage/definition) =="
rg -n "resolveAuthTokenFromCurrentProfile" cli/claude-proxy.js
echo
echo "== Inspect how BUILTIN_CLAUDE_PROXY_SETTINGS_FILE is used (usage sites) =="
rg -n "BUILTIN_CLAUDE_PROXY_SETTINGS_FILE" cli/claude-proxy.jsRepository: SakuraByteCore/codexmate
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli.js: locate proxyToken usage and applyToClaudeSettings write paths =="
rg -n "proxyToken|applyToClaudeSettings|settingsApiKey|settingsBaseUrl|claude.*settings\.json|~\/\.claude\/settings\.json" cli.js
echo
echo "== cli.js: show region around proxyToken + startBuiltinClaudeProxyRuntime call =="
cat -n cli.js | sed -n '9165,9265p'
echo
echo "== cli/claude-proxy.js: show read/save settings helpers =="
cat -n cli/claude-proxy.js | sed -n '640,760p'
echo
echo "== cli/claude-proxy.js: show startBuiltinClaudeProxyRuntime payload merge + controller start =="
cat -n cli/claude-proxy.js | sed -n '1180,1265p'
echo
echo "== cli/claude-proxy.js: show incoming Claude request auth check (CODEXMATE_HTTP_TOKEN / x-codexmate-token) =="
cat -n cli/claude-proxy.js | sed -n '1120,1185p'Repository: SakuraByteCore/codexmate
Length of output: 431
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli.js: find proxyToken + settingsApiKey/settingsBaseUrl usage =="
rg -n "proxyToken" cli.js
rg -n "applyToClaudeSettings" cli.js
rg -n "settingsApiKey|settingsBaseUrl" cli.js
rg -n "claude.*settings\.json|~\/\.claude\/settings\.json" cli.js || trueRepository: SakuraByteCore/codexmate
Length of output: 990
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli.js: show region around proxyToken + startBuiltinClaudeProxyRuntime call =="
cat -n cli.js | sed -n '9170,9250p'Repository: SakuraByteCore/codexmate
Length of output: 3974
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli/claude-proxy.js: show read/save settings helpers and startBuiltinClaudeProxyRuntime payload merge =="
cat -n cli/claude-proxy.js | sed -n '620,820p'
cat -n cli/claude-proxy.js | sed -n '1160,1265p'Repository: SakuraByteCore/codexmate
Length of output: 14937
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cli/claude-proxy.js: show incoming Claude request auth check =="
cat -n cli/claude-proxy.js | sed -n '1120,1185p'Repository: SakuraByteCore/codexmate
Length of output: 3657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS host definition =="
rg -n "DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS" -S cli.js cli/claude-proxy.js
echo
echo "== Show where proxy listen url host comes from =="
rg -n "buildBuiltinClaudeProxyListenUrl|formatHostForUrl|http://\\$\\{host\\}" cli/claude-proxy.js
echo
echo "== Search CLAUDE_SETTINGS_FILE usage in cli/claude-proxy.js =="
rg -n "CLAUDE_SETTINGS_FILE" cli/claude-proxy.js || true
echo
echo "== Show DEFAULT_* settings block (closest match regions) =="
# Print small regions around each DEFAULT_* match
for f in cli.js cli/claude-proxy.js; do
echo "--- $f ---"
rg -n "DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS" "$f" | cut -d: -f1 | sort -u | while read -r line; do
start=$((line-20)); end=$((line+40));
sed -n "${start},${end}p" "$f";
done
doneRepository: SakuraByteCore/codexmate
Length of output: 45075
Fix proxy-token/auth mismatch in chat_completions flow.
applyToClaudeSettings(...) generates proxyToken and writes it into Claude’s ANTHROPIC_API_KEY, but startBuiltinClaudeProxyRuntime(...) doesn’t pass that token anywhere. In cli/claude-proxy.js, the proxy does not use Claude settings to validate it; it only enforces access using process.env.CODEXMATE_HTTP_TOKEN for non-loopback requests (loopback requests bypass this check). With DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host = 127.0.0.1 this often won’t surface, but if Claude reaches the proxy via a non-loopback address, requests will be unauthorized unless CODEXMATE_HTTP_TOKEN matches the generated proxyToken.
🤖 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 `@cli.js` around lines 9192 - 9203, The generated proxyToken from
applyToClaudeSettings(...) is not passed into startBuiltinClaudeProxyRuntime,
causing an auth mismatch; update the call to startBuiltinClaudeProxyRuntime (the
invocation that currently sets host, provider, authSource, targetApi, timeoutMs,
upstreamProviderName, upstreamBaseUrl, upstreamApiKey) to include the generated
proxyToken (e.g., as an authToken or proxyToken parameter) so the runtime
enforces the same token that was written into Claude’s ANTHROPIC_API_KEY, and
also verify cli/claude-proxy.js checks that token (or falls back to
CODEXMATE_HTTP_TOKEN only when appropriate) for non-loopback requests when
DEFAULT_BUILTIN_CLAUDE_PROXY_SETTINGS.host is non-loopback. Ensure parameter
naming matches startBuiltinClaudeProxyRuntime’s signature and update any
doc/comment accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
chat_completionscan explicitly route/v1/messagesto upstream/v1/chat/completions.wire_apisemantics./v1/models, OpenAI bridge upstream resolution, provider auth, and upstream error mapping.Validation
node -c cli/claude-proxy.jsnode -c cli.jsnpm run test:unit(All 533 tests passed)npm run test:e2enpm run lintgit diff --checktelepub,DeepSeek-V4-pro). The supplied key was the redactedsk-Ad2…QfCj, so real upstream health returned provider unreachable; no live telepub completion success is claimed.Notes
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Tests