feat: add configurable messages-path for Claude upstream (fixes #2055)#2063
feat: add configurable messages-path for Claude upstream (fixes #2055)#2063admccc wants to merge 13 commits intorouter-for-me:mainfrom
Conversation
When users configure relay services (e.g., new-api) as Claude upstream, they often set base-url with a trailing /v1 (e.g., https://example.com/v1). The code was appending /v1/messages to the base URL, resulting in double /v1 path (/v1/v1/messages) causing 404 errors. Add normalizeClaudeBaseURL() helper that strips trailing slashes and /v1 suffix before URL construction in Execute, ExecuteStream and CountTokens. Fixes: router-for-me#2055
Add unit tests for the normalizeClaudeBaseURL helper, covering: - plain URL without version suffix (unchanged) - trailing slash removal - /v1 suffix stripping - /v1 with trailing slash - longer paths with /v1 - unrelated paths - empty string
Add claudeMessagesPath() helper that reads messages_path from auth.Attributes, falling back to "/v1/messages" by default. Use this helper in Execute, ExecuteStream, and CountTokens to build upstream URLs. This allows users to configure relay services (e.g., new-api) that expose Claude-compatible APIs at a non-standard path via the new messages-path config field.
Test the new claudeMessagesPath helper for custom path support.
Allow users to customize the API path appended to base-url when making requests to Claude upstream. Defaults to /v1/messages.
When synthesizing Claude API key auth entries, copy the messages_path configuration to auth.Attributes so it is available to the executor.
Add example and explanation for the new messages-path field that allows users to connect relay services without the /v1 prefix.
admccc
changed the title
Add MessagesPath to claudeKeyPatch struct and PatchClaudeKey handler so the field can be set/updated via the management REST API. Also normalise the field in normalizeClaudeKey().
Show the configured messages-path alongside base-url in the provider keys list when the field is non-empty.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new configuration option for Claude API keys, "messages-path", to address connectivity issues with non-standard Claude API relay services. By allowing users to define the specific endpoint path for messages, it significantly improves flexibility and compatibility, ensuring that the application can interact correctly with various Claude API implementations without being constrained by a hardcoded "/v1/messages" path. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
![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
The pull request introduces a normalizeClaudeBaseURL function to standardize Claude API base URLs by removing trailing slashes and the /v1 suffix, which is then applied in Execute, ExecuteStream, and CountTokens methods, along with a new test TestNormalizeClaudeBaseURL. However, a high-severity issue was identified where the TrimSuffix for /v1 is too aggressive; it could incorrectly strip a user-configured /v1 from the base-url when combined with the messages-path feature, potentially leading to an invalid final API endpoint. The suggested improvement is to only trim trailing slashes and allow the final URL construction logic to handle path segments.
| } | ||
| return "/v1/messages" |
There was a problem hiding this comment.
The TrimSuffix call that removes /v1 from the baseURL is too aggressive and may lead to incorrect URLs when combined with the new messages-path feature.
As per the PR description, a user might configure a base-url that includes a version path and use messages-path for the final segment. For instance:
- api-key: "your-relay-key"
base-url: "https://new-api.example.com/v1"
messages-path: "/messages"The expected final URL is https://new-api.example.com/v1/messages. However, this function will strip /v1, resulting in an incorrect URL: https://new-api.example.com/messages.
To ensure user configurations are respected, it would be safer to only trim the trailing slash from the base URL. The logic that constructs the final URL should be responsible for handling path segments correctly.
| } | |
| return "/v1/messages" | |
| baseURL = strings.TrimRight(baseURL, "/") |
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary:
This change moves Claude upstream routing in the right direction, but there are still compatibility gaps in the final URL construction. I found one blocking issue that means the original /v1/v1/messages failure mode is still possible, plus one normalization gap on the new messages-path field.
Key findings:
- Blocking:
internal/runtime/executor/claude_executor.gonow concatenatesbaseURLandmessages-pathdirectly. If a user keeps the common relay configuration from issue #2055 (base-url: https://relay.example.com/v1) and relies on the default path, the final URL is still.../v1/v1/messages. That means the original bug is not actually fixed for existing configs. The new test only checks path selection, not the final URL assembly, so this regression is currently untested. - Non-blocking but important:
messages-pathis onlyTrimSpace'd. Values likemessages,v1/messages, or/messages/will produce malformed URLs such ashttps://hostmessages,https://hostv1/messages, or.../messages//count_tokens. Since this is a new user-facing config field, it should be normalized (at least enforce a leading/and trim a trailing/) and covered by tests.
Test plan:
- Reviewed the final diff in a clean PR worktree.
- Ran
go test ./internal/runtime/executor. - Ran
go test ./internal/watcher/synthesizer ./internal/api/handlers/management ./internal/tui. - Ran
go build -o test-output ./cmd/server && rm test-output.
Code ReviewGood direction on supporting non-standard Claude relay paths. The messages-path config field and claudeMessagesPath() helper are clean and well-tested. IssuesBlocking: URL construction does not fix the original bug The concatenation in claude_executor.go still produces double /v1 for existing configs: url := fmt.Sprintf("%s%s?beta=true", baseURL, claudeMessagesPath(auth))If user has base-url: https://relay.example.com/v1 (from #2055) and uses default messages-path, final URL becomes: The original issue persists. The new test only checks claudeMessagesPath() behavior, not final URL assembly. Non-blocking: Normalize messages-path field messages-path is only TrimSpace-ed, so these malformed inputs will break:
Enforce leading / and trim trailing / during normalization, and add test coverage for URL construction with various base-url + messages-path combos. SummaryApprove with fixes to (1) prevent double /v1 for existing configs and (2) normalize path format. |
Add normalizeBaseURL() to strip trailing /v1 from base URLs, preventing double-path issues when relay services include /v1 in their base URL (e.g., https://relay.example.com/v1 + /v1/messages -> /v1/v1/messages). Also normalize claudeMessagesPath() to ensure the returned path always starts with / and has no trailing /. Fixes: router-for-me#2055
Add comprehensive tests for the new normalizeBaseURL() function covering trailing /v1 stripping, nested paths, and non-matching suffixes. Expand TestClaudeMessagesPath with cases for missing leading slash, trailing slash, and combined normalization scenarios.
…im trailing /) Update normalizeClaudeKey() to enforce a leading slash and strip trailing slashes from messages-path values. This prevents malformed URLs from values like "messages", "v1/messages", or "/messages/".
…trailing /) Apply the same messages-path normalization in the config synthesizer so that auth.Attributes["messages_path"] always holds a canonical path starting with / and without trailing slashes.
3 participants
Summary
Add a configurable
messages-pathfield toClaudeKeyso that relay services exposing Claude-compatible APIs at non-standard paths (e.g., new-api, one-api) can be configured without hardcoding/v1/messages.Root Cause
The executor hardcoded
/v1/messagesappended tobase-url, causing…/v1/v1/messageswhen relay services already include/v1in their base URL (e.g.,https://relay.example.com/v1).Solution
messages-pathconfig field — users can set a custom path per Claude key; defaults to/v1/messages.normalizeBaseURL()— strips trailing/v1frombase-urlbefore combining withmessages-path, preventing the double-path issue automatically.messages-pathvalues are normalized at every entry point (synthesizer, management API): leading/is enforced and trailing/is stripped, so values likemessages,v1/messages, or/messages/all produce correct URLs.Changes
internal/config/config.goMessagesPathfield toClaudeKeyinternal/watcher/synthesizer/config.gomessages-pathinto auth attributesinternal/runtime/executor/claude_executor.gonormalizeBaseURL()+ normalizeclaudeMessagesPath()output; apply to all 3 URL construction sitesinternal/runtime/executor/claude_executor_test.goTestNormalizeBaseURL; expandTestClaudeMessagesPathwith normalization casesinternal/api/handlers/management/config_lists.gomessages-pathinnormalizeClaudeKey()andPatchClaudeKeyinternal/tui/keys_tab.go[path: …]in keys tab whenmessages-pathis setconfig.example.yamlmessages-pathfield with usage examplesExample configuration
Test plan
TestNormalizeBaseURL— covers/v1stripping, nested paths, non-matching suffixesTestClaudeMessagesPath— covers nil auth, missing leading slash, trailing slash, whitespacebase-urlending in/v1produces correct URLFixes: #2055