Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cmd/opencodereview/config_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions cmd/opencodereview/config_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down
24 changes: 24 additions & 0 deletions cmd/opencodereview/provider_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package main
import (
"encoding/json"
"fmt"
"net/url"
"os"
"path/filepath"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
52 changes: 52 additions & 0 deletions cmd/opencodereview/provider_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
7 changes: 6 additions & 1 deletion cmd/opencodereview/provider_tui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
25 changes: 25 additions & 0 deletions cmd/opencodereview/provider_tui_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
58 changes: 58 additions & 0 deletions internal/llm/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions pages/src/content/docs/en/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions pages/src/content/docs/ja/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` と
Expand Down
20 changes: 20 additions & 0 deletions pages/src/content/docs/ru/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 возвращается к
предустановленному значению по умолчанию — поэтому его нужно задавать только
когда ваша конечная точка отличается.

### Пользовательские провайдеры

Любое имя провайдера, которого нет в таблице выше, считается
Expand Down
16 changes: 16 additions & 0 deletions pages/src/content/docs/zh/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Loading