Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors URL construction and sanitization for Claude Code (CC) compatible providers. It updates default chat paths to include the /v1 prefix, adjusts default base URLs, and introduces specialized joining and stripping functions to handle versioned endpoints more robustly. Review feedback suggests updating the default models path for consistency, expanding URL sanitization logic to include the /models endpoint, and reducing code duplication by centralizing sanitization functions in a shared service.
| export const CLAUDE_CODE_COMPATIBLE_PREFIX = "anthropic-compatible-cc-"; | ||
| export const CLAUDE_CODE_COMPATIBLE_DEFAULT_CHAT_PATH = "/messages?beta=true"; | ||
| export const CLAUDE_CODE_COMPATIBLE_DEFAULT_CHAT_PATH = "/v1/messages?beta=true"; | ||
| export const CLAUDE_CODE_COMPATIBLE_DEFAULT_MODELS_PATH = "/models"; |
There was a problem hiding this comment.
The default models path should be updated to /v1/models to be consistent with the updated chat path and the new URL normalization logic. Since stripClaudeCodeCompatibleEndpointSuffix strips the /v1 segment when a full endpoint is provided, using /models as the default will result in an incorrect URL (e.g., .../models instead of .../v1/models) if a user pastes a full messages URL as the base URL.
| export const CLAUDE_CODE_COMPATIBLE_DEFAULT_MODELS_PATH = "/models"; | |
| export const CLAUDE_CODE_COMPATIBLE_DEFAULT_MODELS_PATH = "/v1/models"; |
| const CC_COMPATIBLE_DETAILS_TITLE = "CC Compatible Details"; | ||
| const CC_COMPATIBLE_DEFAULT_CHAT_PATH = "/messages?beta=true"; | ||
| const CC_COMPATIBLE_DEFAULT_CHAT_PATH = "/v1/messages?beta=true"; | ||
| const CC_COMPATIBLE_DEFAULT_MODELS_PATH = "/models"; |
There was a problem hiding this comment.
The default models path should be updated to /v1/models for consistency with the service and to ensure correct URL construction for CC compatible providers.
| const CC_COMPATIBLE_DEFAULT_MODELS_PATH = "/models"; | |
| const CC_COMPATIBLE_DEFAULT_MODELS_PATH = "/v1/models"; |
| const ADD_CC_COMPATIBLE_LABEL = "Add CC Compatible"; | ||
| const CC_COMPATIBLE_DEFAULT_CHAT_PATH = "/messages?beta=true"; | ||
| const CC_COMPATIBLE_DEFAULT_CHAT_PATH = "/v1/messages?beta=true"; | ||
| const CC_COMPATIBLE_DEFAULT_MODELS_PATH = "/models"; |
There was a problem hiding this comment.
The default models path should be updated to /v1/models for consistency with the service and other UI components.
| const CC_COMPATIBLE_DEFAULT_MODELS_PATH = "/models"; | |
| const CC_COMPATIBLE_DEFAULT_MODELS_PATH = "/v1/models"; |
| @@ -45,14 +45,39 @@ export function stripAnthropicMessagesSuffix(baseUrl: string | null | undefined) | |||
| return normalized.replace(/\/messages(?:\?[^#]*)?$/i, ""); | |||
There was a problem hiding this comment.
The generic Anthropic endpoint stripping logic should also handle the /models suffix. This ensures that if a user pastes a models endpoint as the base URL, it is correctly sanitized before being used to construct other endpoints.
| return normalized.replace(/\/messages(?:\?[^#]*)?$/i, ""); | |
| return normalized.replace(/\/(?:messages|models)(?:\?[^#]*)?$/i, ""); |
| .trim() | ||
| .replace(/\/$/, ""); | ||
| if (!normalized) return ""; | ||
| return normalized.replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); |
There was a problem hiding this comment.
The CC-specific endpoint stripping logic should also handle the /models suffix (optionally prefixed with a version segment). This is important for robust URL normalization when users paste various endpoint URLs into the base URL field.
| return normalized.replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); | |
| return normalized.replace(/\/(?:v\d+\/)?(?:messages|models)(?:\?[^#]*)?$/i, ""); |
| function sanitizeAnthropicBaseUrl(baseUrl: string) { | ||
| return (baseUrl || "") | ||
| .trim() | ||
| .replace(/\/$/, "") | ||
| .replace(/\/messages(?:\?[^#]*)?$/i, ""); | ||
| } | ||
|
|
||
| function sanitizeClaudeCodeCompatibleBaseUrl(baseUrl: string) { | ||
| return (baseUrl || "") | ||
| .trim() | ||
| .replace(/\/$/, "") | ||
| .replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); | ||
| } |
There was a problem hiding this comment.
These sanitization functions are duplicated from open-sse/services/claudeCodeCompatible.ts. To ensure consistency and simplify maintenance, they should be imported from the shared service instead of being redefined locally in multiple API route files.
| function sanitizeClaudeCodeCompatibleBaseUrl(baseUrl: string) { | ||
| return (baseUrl || "") | ||
| .trim() | ||
| .replace(/\/$/, "") | ||
| .replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); | ||
| } |
There was a problem hiding this comment.
This function is duplicated from open-sse/services/claudeCodeCompatible.ts. It should be imported from the shared service to maintain consistency across the codebase.
| function sanitizeClaudeCodeCompatibleBaseUrl(baseUrl: string) { | ||
| return (baseUrl || "") | ||
| .trim() | ||
| .replace(/\/$/, "") | ||
| .replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); | ||
| } |
There was a problem hiding this comment.
This function is duplicated from open-sse/services/claudeCodeCompatible.ts. It should be imported from the shared service.
rdself
changed the title
|
Obrigado pela contribuição! 🎉 A funcionalidade/correção foi integrada localmente na branch |
|
Integrated into release/v3.4.4. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| "GET https://proxy.example.com/v1/models", | ||
| "POST https://proxy.example.com/v1/messages?beta=true", | ||
| ] | ||
| ["GET https://proxy.example.com/models", "POST https://proxy.example.com/v1/messages?beta=true"] |
There was a problem hiding this comment.
WARNING: Inconsistent URL construction - the test expects /models without the /v1 prefix while the chat path uses /v1/messages?beta=true. This is the buggy behavior that the existing HIGH priority comments correctly flag. When CLAUDE_CODE_COMPATIBLE_DEFAULT_MODELS_PATH is updated to /v1/models, this test expectation should change to include the /v1 prefix for consistency.
| const data = await response.json(); | ||
| assert.match(data.node.id, /^anthropic-compatible-cc-/); | ||
| assert.equal(data.node.baseUrl, "https://proxy.example.com/v1"); | ||
| assert.equal(data.node.baseUrl, "https://proxy.example.com"); |
There was a problem hiding this comment.
WARNING: This test expects baseUrl without the /v1 segment (https://proxy.example.com instead of https://proxy.example.com/v1). This is correct behavior when the full endpoint URL is pasted and sanitized. However, verify this aligns with the new URL normalization logic - the test passes currently but may need adjustment if models path is updated to include /v1.
Code Review SummaryStatus: 10 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)No issues found in unchanged code. Files Reviewed (10 files)
Analysis Summary: This PR addresses CC Compatible URL normalization issues by splitting generic Anthropic-compatible URL handling from CC-specific handling. The changes are well-intentioned and target the correct root cause. Existing issues are valid:
New issues identified:
Recommendation: The existing comments provide clear suggestions for fixes. Once those are addressed, this PR should be mergeable. |
|
Obrigado pela contribuição! 🎉 A funcionalidade/correção foi integrada localmente na branch |
There was a problem hiding this comment.
Pull request overview
Fixes Claude Code (CC) compatible provider URL handling so CC endpoints reliably resolve to /v1/messages?beta=true while keeping generic Anthropic-compatible normalization unchanged.
Changes:
- Restore CC default chat path to
/v1/messages?beta=trueand introduce CC-specific URL joining/stripping helpers. - Update validation + runtime URL building to use CC-specific normalization (
joinClaudeCodeCompatibleUrl,stripClaudeCodeCompatibleEndpointSuffix). - Align API/UI create/update/validate flows and add unit tests covering CC URL joins and message-shape edge cases.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/cc-compatible-provider.test.mjs | Adds/updates tests for CC URL joining, executor URL building, validation call ordering, and message fallback behavior. |
| src/lib/providers/validation.ts | Splits Anthropic vs CC base URL normalization; uses CC join helper for /models + /v1/messages?beta=true validation probes. |
| src/app/api/provider-nodes/validate/route.ts | Uses CC-specific base URL sanitization when validating CC nodes. |
| src/app/api/provider-nodes/route.ts | Sanitizes CC base URLs differently during create; routes CC vs generic Anthropic normalization. |
| src/app/api/provider-nodes/[id]/route.ts | Applies CC vs generic Anthropic base URL sanitization during updates. |
| src/app/(dashboard)/dashboard/providers/page.tsx | Updates CC defaults in “Add CC Compatible” flow (base URL + chat path). |
| src/app/(dashboard)/dashboard/providers/[id]/page.tsx | Updates CC defaults in edit flow (base URL + chat path). |
| open-sse/services/provider.ts | Uses CC-specific URL joiner for CC provider runtime URL construction. |
| open-sse/services/claudeCodeCompatible.ts | Restores CC default chat path, adds CC strip/join helpers, and adjusts CC message building behavior. |
| open-sse/executors/default.ts | Uses CC-specific URL joiner when building CC runtime URLs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const rawBaseUrl = baseUrl || ANTHROPIC_COMPATIBLE_DEFAULTS.baseUrl; | ||
| const sanitizedBaseUrl = | ||
| compatMode === "cc" | ||
| ? sanitizeClaudeCodeCompatibleBaseUrl(rawBaseUrl) |
There was a problem hiding this comment.
sanitizeClaudeCodeCompatibleBaseUrl() only strips a trailing /messages (and optional /vN/messages), but leaves a bare /v1 intact. That means creating a CC node with no baseUrl (or with baseUrl: https://api.anthropic.com/v1) will persist .../v1, and later /models validation will be built as .../v1/models rather than the intended root .../models. Consider using a CC-specific default base URL (e.g., https://api.anthropic.com) and/or stripping a trailing /v\d+ segment for CC mode so pasted /v1 inputs normalize to the same stored base URL as root inputs.
| ? sanitizeClaudeCodeCompatibleBaseUrl(rawBaseUrl) | |
| ? sanitizeClaudeCodeCompatibleBaseUrl(rawBaseUrl.replace(/\/v\d+\/?$/i, "")) |
| export function stripClaudeCodeCompatibleEndpointSuffix( | ||
| baseUrl: string | null | undefined | ||
| ): string { | ||
| const normalized = String(baseUrl || "") | ||
| .trim() | ||
| .replace(/\/$/, ""); | ||
| if (!normalized) return ""; | ||
| return normalized.replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); | ||
| } | ||
|
|
||
| function joinNormalizedBaseUrlAndPath(baseUrl: string, path: string): string { | ||
| const normalizedBase = String(baseUrl || "").replace(/\/$/, ""); | ||
| const normalizedPath = String(path || "").startsWith("/") | ||
| ? String(path) | ||
| : `/${String(path || "")}`; | ||
| const versionMatch = normalizedBase.match(/(\/v\d+)$/i); | ||
| if ( | ||
| versionMatch && | ||
| normalizedPath.toLowerCase().startsWith(`${versionMatch[1].toLowerCase()}/`) | ||
| ) { | ||
| return `${normalizedBase}${normalizedPath.slice(versionMatch[1].length)}`; | ||
| } | ||
| return `${normalizedBase}${normalizedPath}`; | ||
| } | ||
|
|
||
| export function joinBaseUrlAndPath(baseUrl: string, path: string): string { | ||
| return joinNormalizedBaseUrlAndPath(stripAnthropicMessagesSuffix(baseUrl), path); | ||
| } | ||
|
|
||
| export function joinClaudeCodeCompatibleUrl(baseUrl: string, path: string): string { | ||
| return joinNormalizedBaseUrlAndPath(stripClaudeCodeCompatibleEndpointSuffix(baseUrl), path); | ||
| } |
There was a problem hiding this comment.
joinClaudeCodeCompatibleUrl() preserves a trailing /vN on the base URL, which can change CC "root" endpoints. For example, joinClaudeCodeCompatibleUrl('https://proxy.example.com/v1', '/models') currently yields https://proxy.example.com/v1/models, but the CC flow (and tests in this PR) expects models probing at the root /models. To match the stated normalization goals (accept base URLs pasted as root, /v1, or full /v1/messages?...), consider updating CC base URL normalization (e.g., stripClaudeCodeCompatibleEndpointSuffix) to also strip a trailing /v\d+, or otherwise ensure /models is always joined against the versionless root.
| function sanitizeClaudeCodeCompatibleBaseUrl(baseUrl: string) { | ||
| return (baseUrl || "") | ||
| .trim() | ||
| .replace(/\/$/, "") | ||
| .replace(/\/(?:v\d+\/)?messages(?:\?[^#]*)?$/i, ""); | ||
| } |
There was a problem hiding this comment.
The CC base URL sanitization logic is now duplicated in multiple API routes (this file, src/app/api/provider-nodes/validate/route.ts, and src/app/api/provider-nodes/[id]/route.ts) while similar normalization also exists in open-sse/services/claudeCodeCompatible.ts (stripClaudeCodeCompatibleEndpointSuffix). This duplication increases the risk that create/update/validate/runtime will drift again. Consider extracting a single shared sanitizer (e.g., in a shared URL util) and reusing it across the routes and validation helpers.
Summary
/v1/messages?beta=true/modelsand/v1/messages?beta=truecorrectlyRoot Cause
The CC Compatible flow on the latest
mainwas normalizing provider URLs like a generic Anthropic-compatible endpoint.That broke the hidden CC bridge in two ways:
/messages?beta=trueinstead of/v1/messages?beta=true/v1/messages?beta=truewere not normalized consistently across create, update, validation, and runtime executionThe failing log that triggered this fix showed OmniRoute calling
https://www.xxx.com/messages?beta=true, getting the site's HTML homepage back, and then surfacing that asInvalid JSON response from provider, which bubbled up as a502.Impact
/v1/messages?beta=trueendpoint by default/v1/modelsand/v1/messagesprobing behavior/v1, or a full/v1/messages?beta=trueURL and still get correct runtime and validation behaviorValidation
node --import tsx/esm --test tests/unit/cc-compatible-provider.test.mjs tests/unit/provider-validation-branches.test.mjsnpx eslint 'open-sse/services/claudeCodeCompatible.ts' 'open-sse/executors/default.ts' 'open-sse/services/provider.ts' 'src/lib/providers/validation.ts' 'src/app/api/provider-nodes/route.ts' 'src/app/api/provider-nodes/[id]/route.ts' 'src/app/api/provider-nodes/validate/route.ts' 'src/app/(dashboard)/dashboard/providers/page.tsx' 'src/app/(dashboard)/dashboard/providers/[id]/page.tsx' 'tests/unit/cc-compatible-provider.test.mjs'npm run test:unitpassed locally duringgit commit(1233tests)