mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-08-15 19:54:11 +00:00
feat(provider): support custom Base URL for LiteLLM/built-in providers (#729)
* feat(provider): add editable Base URL step to official provider wizard The official-provider tab in `ocr config provider` only captured API key and model, with no way to override a preset provider's Base URL. The resolver already honored `entry.URL` over `preset.BaseURL`, but the TUI never exposed it — litellm (a self-hosted gateway rarely at http://localhost:4000/v1) was the canonical pain point. Add a Base URL step to the official-tab flow (stepModel -> stepBaseURL -> stepAPIKey), pre-filled with the effective URL (configured override or preset default). Persist `providers.<name>.url` only when the entered value differs from the preset default, so the preset remains the fallback and configs without an explicit url are unchanged. Custom/manual tabs are unaffected. Add resolver regression tests (litellm override + default fallback) and TUI tests (pre-fill with preset/override, Esc navigation, persistence of override vs. clearing on preset default). Update the four official-tab tests that assumed stepModel -> stepAPIKey to traverse the new step. * feat(provider): surface override Base URL in model picker and document it With the wizard now able to set a Base URL override for built-in providers, make the override visible and discoverable. - `ocr config model` shows the effective Base URL for a preset provider (the configured `providers.<name>.url` override, or the preset default when none is set) so users can confirm their gateway is in use. - The provider-wizard model-selection step shows the same effective URL via a tab-aware `effectiveBaseURL()` helper (official override/preset, or custom provider URL). - Document `providers.<name>.url` as a built-in provider override in the configuration docs, with a litellm example and the preset-as-default semantics; note the wizard's editable Base URL step. Add tests covering the model-selector display (override vs preset default) and the wizard's effectiveBaseURL resolution. * fix(provider): address PR review — URL trim, validation, dead code, Esc display Address 4 of 5 code review findings on PR #729: 1. URL trim consistency (provider_cmd.go): trim the Base URL once and use the trimmed value for both comparison and persistence, preventing whitespace-polluted URLs from being written to config. 2. URL format validation (provider_cmd.go): validate that the Base URL has an http/https scheme and non-empty host before persisting, giving immediate feedback instead of a runtime failure. Rejects malformed values like bare hosts or ftp:// schemes. 3. Dead code removal (provider_tui.go): remove the init-time pre-fill of officialURLInput that is always overwritten by loadOfficialURL() when the user enters the Base URL step. Pre-fill logic now lives in a single place. 4. effectiveBaseURL reflects pending edit (provider_tui.go): when the user edits the Base URL and presses Esc back to model selection, effectiveBaseURL() now returns the in-progress value from officialURLInput instead of the stale on-disk config. The SSRF/private-IP finding (#2 in review) is not addressed — it is a false positive for a local CLI tool where localhost and private network endpoints are the primary use case (the litellm preset default is http://localhost:4000/v1). * feat(provider): implement URL trimming and validation for provider configuration * fix(provider): remove obsolete official URL handling --------- Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
This commit is contained in:
parent
51750bfe02
commit
6546da9885
11 changed files with 265 additions and 2 deletions
|
|
@ -556,7 +556,13 @@ func applyProviderField(entry *ProviderEntry, field, key, value string) error {
|
|||
case "api_key":
|
||||
entry.APIKey = value
|
||||
case "url":
|
||||
entry.URL = value
|
||||
trimmedURL := strings.TrimSpace(value)
|
||||
if trimmedURL != "" {
|
||||
if err := validateBaseURL(trimmedURL); err != nil {
|
||||
return fmt.Errorf("invalid URL for %s: %w", key, err)
|
||||
}
|
||||
}
|
||||
entry.URL = trimmedURL
|
||||
case "protocol":
|
||||
normalized := llm.NormalizeProtocol(value)
|
||||
if err := llm.ValidateProtocol(normalized); err != nil {
|
||||
|
|
|
|||
|
|
@ -45,6 +45,27 @@ func TestSetConfigValueProvider(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestSetConfigValueProviderURLTrimsAndValidates(t *testing.T) {
|
||||
t.Run("trims a valid URL before storing", func(t *testing.T) {
|
||||
cfg := &Config{}
|
||||
|
||||
if err := setConfigValue(cfg, "providers.litellm.url", " https://gateway.internal:8000/v1 "); err != nil {
|
||||
t.Fatalf("setConfigValue: %v", err)
|
||||
}
|
||||
if got := cfg.Providers["litellm"].URL; got != "https://gateway.internal:8000/v1" {
|
||||
t.Errorf("URL = %q, want trimmed URL", got)
|
||||
}
|
||||
})
|
||||
|
||||
for _, value := range []string{"api.example.com/v1", "ftp://gateway.internal/v1"} {
|
||||
t.Run("rejects "+value, func(t *testing.T) {
|
||||
if err := setConfigValue(&Config{}, "providers.litellm.url", value); err == nil {
|
||||
t.Fatalf("setConfigValue accepted invalid URL %q", value)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetConfigValueModel(t *testing.T) {
|
||||
cfg := &Config{}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ package main
|
|||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
|
|
@ -314,6 +315,12 @@ func runConfigModel() error {
|
|||
if entry, ok := cfg.Providers[cfg.Provider]; ok {
|
||||
currentModel = activeModelForProvider(cfg, cfg.Provider, entry)
|
||||
provider.Models = mergeModelLists(provider.Models, entry.Models)
|
||||
// Surface the effective Base URL: a configured override takes
|
||||
// precedence over the preset default so users can confirm their
|
||||
// gateway is in use from the model picker.
|
||||
if entry.URL != "" {
|
||||
provider.BaseURL = entry.URL
|
||||
}
|
||||
}
|
||||
} else {
|
||||
isCustom = true
|
||||
|
|
@ -412,3 +419,20 @@ func maskKey(key string) string {
|
|||
}
|
||||
return key[:4] + "***" + key[len(key)-4:]
|
||||
}
|
||||
|
||||
// validateBaseURL checks that a provider Base URL has an http or https scheme
|
||||
// and a non-empty host, giving the user immediate feedback rather than
|
||||
// a runtime failure when the LLM client tries to use it.
|
||||
func validateBaseURL(raw string) error {
|
||||
parsed, err := url.Parse(raw)
|
||||
if err != nil {
|
||||
return fmt.Errorf("invalid Base URL %q: %w", raw, err)
|
||||
}
|
||||
if parsed.Scheme != "http" && parsed.Scheme != "https" {
|
||||
return fmt.Errorf("Base URL must use http or https scheme, got %q", parsed.Scheme)
|
||||
}
|
||||
if parsed.Host == "" {
|
||||
return fmt.Errorf("Base URL %q must include a host", raw)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -379,3 +379,55 @@ func TestPrintWizardCancelled(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplyOfficialProviderConfig_PreservesURLWhenWizardOmitsURL verifies that
|
||||
// the URL configured through `ocr config set` survives a later provider wizard
|
||||
// confirmation, whose official flow no longer edits Base URL.
|
||||
func TestApplyOfficialProviderConfig_PreservesURLWhenWizardOmitsURL(t *testing.T) {
|
||||
t.Setenv("LITELLM_API_KEY", "sk-litellm")
|
||||
dir := t.TempDir()
|
||||
configPath := filepath.Join(dir, "config.json")
|
||||
wantURL := "https://old-gateway.internal:9000/v1"
|
||||
cfg := &Config{
|
||||
Providers: map[string]ProviderEntry{
|
||||
"litellm": {URL: wantURL},
|
||||
},
|
||||
}
|
||||
|
||||
err := applyOfficialProviderConfig(configPath, cfg, providerTUIResult{
|
||||
provider: "litellm",
|
||||
model: "openai/gpt-5.4",
|
||||
apiKey: "sk-litellm",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("applyOfficialProviderConfig: %v", err)
|
||||
}
|
||||
if got := cfg.Providers["litellm"].URL; got != wantURL {
|
||||
t.Errorf("persisted URL = %q, want existing override %q", got, wantURL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyOfficialProviderConfig_IgnoresURLFromResult(t *testing.T) {
|
||||
t.Setenv("LITELLM_API_KEY", "sk-litellm")
|
||||
dir := t.TempDir()
|
||||
configPath := filepath.Join(dir, "config.json")
|
||||
wantURL := "https://configured-gateway.internal:9000/v1"
|
||||
cfg := &Config{
|
||||
Providers: map[string]ProviderEntry{
|
||||
"litellm": {URL: wantURL},
|
||||
},
|
||||
}
|
||||
|
||||
err := applyOfficialProviderConfig(configPath, cfg, providerTUIResult{
|
||||
provider: "litellm",
|
||||
model: "openai/gpt-5.4",
|
||||
apiKey: "sk-litellm",
|
||||
url: "https://stale-result.internal:8000/v1",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("applyOfficialProviderConfig: %v", err)
|
||||
}
|
||||
if got := cfg.Providers["litellm"].URL; got != wantURL {
|
||||
t.Errorf("persisted URL = %q, want existing URL %q", got, wantURL)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2905,7 +2905,12 @@ func (m modelTUIModel) View() tea.View {
|
|||
var s strings.Builder
|
||||
s.WriteString("\n")
|
||||
s.WriteString(tuiTitleStyle.Render(fmt.Sprintf(" Select a model (%s)", m.provider.DisplayName)))
|
||||
s.WriteString("\n\n")
|
||||
s.WriteString("\n")
|
||||
if m.provider.BaseURL != "" {
|
||||
s.WriteString(tuiDimStyle.Render(fmt.Sprintf(" Base URL: %s", m.provider.BaseURL)))
|
||||
s.WriteString("\n")
|
||||
}
|
||||
s.WriteString("\n")
|
||||
|
||||
models := m.displayModels()
|
||||
for i, model := range models {
|
||||
|
|
|
|||
|
|
@ -1993,3 +1993,28 @@ func TestProviderTUIView_StepModel_CustomTabDeleteHelp(t *testing.T) {
|
|||
t.Errorf("custom model row should show d Delete hint; got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestModelTUI_ShowsEffectiveBaseURL verifies that the model picker displays
|
||||
// the effective Base URL when a configured override is set on the provider.
|
||||
func TestModelTUI_ShowsEffectiveBaseURL(t *testing.T) {
|
||||
preset, _ := llm.LookupProvider("litellm")
|
||||
preset.BaseURL = "https://gateway.internal:8000/v1"
|
||||
m := newModelTUI(preset, "openai/gpt-5.4")
|
||||
|
||||
got := stripANSI(m.View().Content)
|
||||
if !strings.Contains(got, "Base URL: https://gateway.internal:8000/v1") {
|
||||
t.Errorf("model picker view should show Base URL; got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestModelTUI_ShowsPresetBaseURLWhenNoOverride verifies that the model picker
|
||||
// shows the preset Base URL when no override is configured.
|
||||
func TestModelTUI_ShowsPresetBaseURLWhenNoOverride(t *testing.T) {
|
||||
preset, _ := llm.LookupProvider("litellm")
|
||||
m := newModelTUI(preset, "openai/gpt-5.4")
|
||||
|
||||
got := stripANSI(m.View().Content)
|
||||
if !strings.Contains(got, "Base URL: http://localhost:4000/v1") {
|
||||
t.Errorf("model picker view should show preset Base URL; got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2256,6 +2256,64 @@ func TestEnsureMessagesSuffix(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestResolveEndpoint_PresetProviderURLOverride verifies that a configured
|
||||
// providers.<name>.url overrides the preset BaseURL for a built-in provider,
|
||||
// while the same provider without a url field falls back to preset.BaseURL.
|
||||
// litellm is the canonical case: a self-hosted gateway whose URL is rarely the
|
||||
// preset default (http://localhost:4000/v1).
|
||||
func TestResolveEndpoint_PresetProviderURLOverride(t *testing.T) {
|
||||
clearAllEnv(t)
|
||||
|
||||
cfg := configFile{
|
||||
Provider: "litellm",
|
||||
Providers: map[string]providerEntryConfig{
|
||||
"litellm": {APIKey: "sk-litellm-test", Model: "openai/gpt-5.4", URL: "https://gateway.internal:8000/v1"},
|
||||
},
|
||||
}
|
||||
data, _ := json.Marshal(cfg)
|
||||
cfgPath := filepath.Join(t.TempDir(), "config.json")
|
||||
if err := os.WriteFile(cfgPath, data, 0644); err != nil {
|
||||
t.Fatalf("write config: %v", err)
|
||||
}
|
||||
|
||||
ep, err := ResolveEndpoint(cfgPath)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if ep.URL != "https://gateway.internal:8000/v1" {
|
||||
t.Errorf("URL = %q, want %q (configured url should override preset default)", ep.URL, "https://gateway.internal:8000/v1")
|
||||
}
|
||||
if ep.Protocol != ProtocolOpenAIChatCompletions {
|
||||
t.Errorf("Protocol = %q, want %q", ep.Protocol, ProtocolOpenAIChatCompletions)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveEndpoint_PresetProviderURLDefaultsToPreset verifies that a
|
||||
// built-in provider without a configured url resolves to preset.BaseURL.
|
||||
func TestResolveEndpoint_PresetProviderURLDefaultsToPreset(t *testing.T) {
|
||||
clearAllEnv(t)
|
||||
|
||||
cfg := configFile{
|
||||
Provider: "litellm",
|
||||
Providers: map[string]providerEntryConfig{
|
||||
"litellm": {APIKey: "sk-litellm-test", Model: "openai/gpt-5.4"},
|
||||
},
|
||||
}
|
||||
data, _ := json.Marshal(cfg)
|
||||
cfgPath := filepath.Join(t.TempDir(), "config.json")
|
||||
if err := os.WriteFile(cfgPath, data, 0644); err != nil {
|
||||
t.Fatalf("write config: %v", err)
|
||||
}
|
||||
|
||||
ep, err := ResolveEndpoint(cfgPath)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if ep.URL != "http://localhost:4000/v1" {
|
||||
t.Errorf("URL = %q, want %q (preset default should be used when no url configured)", ep.URL, "http://localhost:4000/v1")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseRetryCodes(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
|
|||
|
|
@ -63,6 +63,24 @@ environment variable.
|
|||
| `siliconflow-cn` | openai | `https://api.siliconflow.cn/v1` | `SILICONFLOW_API_KEY` |
|
||||
| `novita` | openai | `https://api.novita.ai/openai` | `NOVITA_API_KEY` |
|
||||
|
||||
### Overriding a built-in provider's Base URL
|
||||
|
||||
Every built-in provider has a preset Base URL (shown in the table above).
|
||||
To point a built-in provider at a different endpoint — for example a
|
||||
self-hosted LiteLLM gateway that is rarely at the preset default
|
||||
`http://localhost:4000/v1` — set `providers.<name>.url`:
|
||||
|
||||
```bash
|
||||
ocr config set provider litellm
|
||||
ocr config set model openai/gpt-5.4
|
||||
ocr config set providers.litellm.api_key "$LITELLM_API_KEY"
|
||||
ocr config set providers.litellm.url https://gateway.internal:8000/v1
|
||||
```
|
||||
|
||||
The configured `url` takes precedence over the preset Base URL. When
|
||||
`providers.<name>.url` is unset (or cleared), OCR falls back to the
|
||||
preset default — so you only need to set it when your endpoint differs.
|
||||
|
||||
### Custom providers
|
||||
|
||||
Any provider name not in the table above is treated as custom and must
|
||||
|
|
|
|||
|
|
@ -61,6 +61,24 @@ ocr config set providers.anthropic.api_key sk-ant-xxxxxxxxxx
|
|||
| `siliconflow-cn` | openai | `https://api.siliconflow.cn/v1` | `SILICONFLOW_API_KEY` |
|
||||
| `novita` | openai | `https://api.novita.ai/openai` | `NOVITA_API_KEY` |
|
||||
|
||||
### 組み込み provider の Base URL を上書きする
|
||||
|
||||
各組み込み provider にはプリセット Base URL があります(上表を参照)。
|
||||
組み込み provider を別のエンドポイントに向けるには——例えば、プリセット
|
||||
デフォルト `http://localhost:4000/v1` とは異なることが多い自前 LiteLLM
|
||||
ゲートウェイなど——`providers.<name>.url` を設定します:
|
||||
|
||||
```bash
|
||||
ocr config set provider litellm
|
||||
ocr config set model openai/gpt-5.4
|
||||
ocr config set providers.litellm.api_key "$LITELLM_API_KEY"
|
||||
ocr config set providers.litellm.url https://gateway.internal:8000/v1
|
||||
```
|
||||
|
||||
設定した `url` はプリセット Base URL より優先されます。
|
||||
`providers.<name>.url` が未設定(または削除)の場合、OCR はプリセット
|
||||
デフォルトにフォールバックします——エンドポイントが異なる場合のみ設定すればよいです。
|
||||
|
||||
### カスタム provider
|
||||
|
||||
上記の表にない provider 名はすべてカスタムとみなされ、少なくとも `url` と
|
||||
|
|
|
|||
|
|
@ -66,6 +66,26 @@ API-ключ. Если `providers.<name>.api_key` не задан, OCR испо
|
|||
| `siliconflow-cn` | openai | `https://api.siliconflow.cn/v1` | `SILICONFLOW_API_KEY` |
|
||||
| `novita` | openai | `https://api.novita.ai/openai` | `NOVITA_API_KEY` |
|
||||
|
||||
### Переопределение Base URL встроенного провайдера
|
||||
|
||||
У каждого встроенного провайдера есть предустановленный Base URL
|
||||
(см. таблицу выше). Чтобы направить встроенный провайдер на другую конечную
|
||||
точку — например, на собственный шлюз LiteLLM, который редко находится по
|
||||
предустановленному адресу `http://localhost:4000/v1` — задайте
|
||||
`providers.<name>.url`:
|
||||
|
||||
```bash
|
||||
ocr config set provider litellm
|
||||
ocr config set model openai/gpt-5.4
|
||||
ocr config set providers.litellm.api_key "$LITELLM_API_KEY"
|
||||
ocr config set providers.litellm.url https://gateway.internal:8000/v1
|
||||
```
|
||||
|
||||
Заданный `url` имеет приоритет над предустановленным Base URL. Если
|
||||
`providers.<name>.url` не задан (или очищен), OCR возвращается к
|
||||
предустановленному значению по умолчанию — поэтому его нужно задавать только
|
||||
когда ваша конечная точка отличается.
|
||||
|
||||
### Пользовательские провайдеры
|
||||
|
||||
Любое имя провайдера, которого нет в таблице выше, считается
|
||||
|
|
|
|||
|
|
@ -60,6 +60,22 @@ ocr config set providers.anthropic.api_key sk-ant-xxxxxxxxxx
|
|||
| `siliconflow-cn` | openai | `https://api.siliconflow.cn/v1` | `SILICONFLOW_API_KEY` |
|
||||
| `novita` | openai | `https://api.novita.ai/openai` | `NOVITA_API_KEY` |
|
||||
|
||||
### 覆盖内置 provider 的 Base URL
|
||||
|
||||
每个内置 provider 都有一个预设 Base URL(见上表)。要将内置 provider
|
||||
指向不同的端点——例如自建的 LiteLLM 网关,其地址很少是预设默认值
|
||||
`http://localhost:4000/v1`——设置 `providers.<name>.url`:
|
||||
|
||||
```bash
|
||||
ocr config set provider litellm
|
||||
ocr config set model openai/gpt-5.4
|
||||
ocr config set providers.litellm.api_key "$LITELLM_API_KEY"
|
||||
ocr config set providers.litellm.url https://gateway.internal:8000/v1
|
||||
```
|
||||
|
||||
配置的 `url` 优先于预设 Base URL。当 `providers.<name>.url` 未设置(或
|
||||
被清除)时,OCR 回退到预设默认值——因此只需在端点不同时才设置。
|
||||
|
||||
### 自定义 provider
|
||||
|
||||
任何不在上表中的 provider 名都视为自定义,至少要提供 `url` 和 `protocol`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue