diff --git a/cmd/gateway_providers.go b/cmd/gateway_providers.go index 60164088..c3e4418d 100644 --- a/cmd/gateway_providers.go +++ b/cmd/gateway_providers.go @@ -284,12 +284,13 @@ func registerProvidersFromDB(registry *providers.Registry, provStore store.Provi continue } // Local Ollama requires no API key — handle before the key guard (same pattern as ClaudeCLI). + // api_base is stored with /v1 (normalized at write time), so no suffix appending needed. if p.ProviderType == store.ProviderOllama { host := p.APIBase if host == "" { - host = "http://localhost:11434" + host = "http://localhost:11434/v1" } - registry.RegisterForTenant(p.TenantID, providers.NewOpenAIProvider(p.Name, "ollama", config.DockerLocalhost(host+"/v1"), "llama3.3")) + registry.RegisterForTenant(p.TenantID, providers.NewOpenAIProvider(p.Name, "ollama", config.DockerLocalhost(host), "llama3.3")) slog.Info("registered provider from DB", "name", p.Name) continue } diff --git a/internal/http/providers.go b/internal/http/providers.go index 560ff506..944320c7 100644 --- a/internal/http/providers.go +++ b/internal/http/providers.go @@ -76,14 +76,24 @@ func (h *ProvidersHandler) SetAgentStore(as store.AgentCRUDStore) { } // resolveAPIBase returns the provider's api_base, falling back to config/env if empty. +// For Ollama/OllamaCloud providers, applies a safety-net normalization: if the stored +// value is missing the /v1 suffix (pre-existing record before write-time normalization), +// the suffix is appended so all downstream call sites receive a ready-to-use URL. func (h *ProvidersHandler) resolveAPIBase(p *store.LLMProviderData) string { + base := "" if p.APIBase != "" { - return p.APIBase + base = p.APIBase + } else if h.apiBaseFallback != nil { + base = h.apiBaseFallback(p.ProviderType) } - if h.apiBaseFallback != nil { - return h.apiBaseFallback(p.ProviderType) + // Safety net: normalize Ollama URLs missing /v1 (pre-existing DB records). + if base != "" && (p.ProviderType == store.ProviderOllama || p.ProviderType == store.ProviderOllamaCloud) { + base = strings.TrimRight(base, "/") + if !strings.HasSuffix(base, "/v1") { + base += "/v1" + } } - return "" + return base } // emitProviderCacheInvalidate broadcasts a provider cache invalidation event. @@ -164,12 +174,13 @@ func (h *ProvidersHandler) registerInMemory(p *store.LLMProviderData) { } // Ollama doesn't need an API key — handle before the key guard (same as startup). // In Docker, swap localhost → host.docker.internal so the container can reach the host. + // api_base is stored with /v1 (normalized at write time), so no suffix appending needed. if p.ProviderType == store.ProviderOllama { host := p.APIBase if host == "" { - host = "http://localhost:11434" + host = "http://localhost:11434/v1" } - h.providerReg.RegisterForTenant(p.TenantID, providers.NewOpenAIProvider(p.Name, "ollama", config.DockerLocalhost(host+"/v1"), "llama3.3")) + h.providerReg.RegisterForTenant(p.TenantID, providers.NewOpenAIProvider(p.Name, "ollama", config.DockerLocalhost(host), "llama3.3")) return } if p.APIKey == "" { @@ -210,6 +221,22 @@ func (h *ProvidersHandler) registerInMemory(p *store.LLMProviderData) { } } +// normalizeOllamaAPIBase ensures Ollama and OllamaCloud api_base values include the +// /v1 suffix required for OpenAI-compatible endpoints. Normalizing at write time means +// resolveAPIBase() always returns a ready-to-use base URL. +func normalizeOllamaAPIBase(p *store.LLMProviderData) { + if p.ProviderType != store.ProviderOllama && p.ProviderType != store.ProviderOllamaCloud { + return + } + if p.APIBase == "" { + return + } + p.APIBase = strings.TrimRight(p.APIBase, "/") + if !strings.HasSuffix(p.APIBase, "/v1") { + p.APIBase += "/v1" + } +} + // validateProviderURL rejects provider base URLs pointing to internal/private networks. // Defense-in-depth: prevents SSRF when providers are later used for API calls. func validateProviderURL(rawURL string, providerType string) error { @@ -321,6 +348,10 @@ func (h *ProvidersHandler) handleCreateProvider(w http.ResponseWriter, r *http.R return } + // Normalize Ollama base URL to include /v1 so all code paths + // (chat, model listing, embedding verify) use the same value from DB. + normalizeOllamaAPIBase(&p) + if err := h.store.CreateProvider(r.Context(), &p); err != nil { slog.Error("providers.create", "error", err) writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) @@ -447,6 +478,12 @@ func (h *ProvidersHandler) handleUpdateProvider(w http.ResponseWriter, r *http.R } } + // Normalize Ollama base URL to include /v1 so all code paths use the same value. + normalizeOllamaAPIBase(&candidate) + if candidate.APIBase != currentProvider.APIBase { + updates["api_base"] = candidate.APIBase + } + if err := validateChatGPTOAuthProviderCandidate(r.Context(), h.store, id, &candidate); err != nil { writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) return diff --git a/internal/http/providers_ollama_url_test.go b/internal/http/providers_ollama_url_test.go new file mode 100644 index 00000000..cac5e1b1 --- /dev/null +++ b/internal/http/providers_ollama_url_test.go @@ -0,0 +1,274 @@ +package http + +import ( + "testing" + + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/providers" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// TestNormalizeOllamaAPIBase validates write-time normalization applied before DB persist. +// The function must append /v1, strip redundant trailing slashes, and leave non-Ollama +// provider types untouched. +func TestNormalizeOllamaAPIBase(t *testing.T) { + tests := []struct { + name string + providerType string + input string + want string + }{ + // --- Ollama normalization cases --- + { + name: "bare host:port gets /v1 appended", + providerType: store.ProviderOllama, + input: "http://host:11434", + want: "http://host:11434/v1", + }, + { + name: "trailing slash is stripped before /v1 is appended", + providerType: store.ProviderOllama, + input: "http://host:11434/", + want: "http://host:11434/v1", + }, + { + name: "already-normalized /v1 is not doubled", + providerType: store.ProviderOllama, + input: "http://host:11434/v1", + want: "http://host:11434/v1", + }, + { + name: "trailing slash after /v1 is stripped", + providerType: store.ProviderOllama, + input: "http://host:11434/v1/", + want: "http://host:11434/v1", + }, + { + name: "empty api_base is left unchanged", + providerType: store.ProviderOllama, + input: "", + want: "", + }, + // --- OllamaCloud also normalized --- + { + name: "OllamaCloud bare URL gets /v1", + providerType: store.ProviderOllamaCloud, + input: "https://ollama.com", + want: "https://ollama.com/v1", + }, + { + name: "OllamaCloud already-normalized /v1 unchanged", + providerType: store.ProviderOllamaCloud, + input: "https://ollama.com/v1", + want: "https://ollama.com/v1", + }, + // --- Non-Ollama types must NOT be touched --- + { + name: "OpenAI-compat is not modified", + providerType: store.ProviderOpenAICompat, + input: "http://host:11434", + want: "http://host:11434", + }, + { + name: "Anthropic native is not modified", + providerType: store.ProviderAnthropicNative, + input: "https://api.anthropic.com", + want: "https://api.anthropic.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &store.LLMProviderData{ + ProviderType: tt.providerType, + APIBase: tt.input, + } + normalizeOllamaAPIBase(p) + if p.APIBase != tt.want { + t.Fatalf("normalizeOllamaAPIBase() APIBase = %q, want %q", p.APIBase, tt.want) + } + }) + } +} + +// TestResolveAPIBaseSafetyNet validates the read-time safety net in resolveAPIBase. +// Pre-existing DB records (written before write-time normalization) must receive /v1 +// on read; already-normalized values and non-Ollama types must be returned as-is. +func TestResolveAPIBaseSafetyNet(t *testing.T) { + tests := []struct { + name string + providerType string + apiBase string + want string + }{ + // Ollama missing /v1 — safety net fires + { + name: "Ollama bare host:port gets /v1 via safety net", + providerType: store.ProviderOllama, + apiBase: "http://host:11434", + want: "http://host:11434/v1", + }, + { + name: "Ollama trailing slash gets /v1 via safety net", + providerType: store.ProviderOllama, + apiBase: "http://host:11434/", + want: "http://host:11434/v1", + }, + // Ollama already normalized — no double /v1 + { + name: "Ollama already has /v1 — no double append", + providerType: store.ProviderOllama, + apiBase: "http://host:11434/v1", + want: "http://host:11434/v1", + }, + { + name: "Ollama /v1 with trailing slash — trimmed, no double", + providerType: store.ProviderOllama, + apiBase: "http://host:11434/v1/", + want: "http://host:11434/v1", + }, + // Non-Ollama — must never have /v1 appended by safety net + { + name: "OpenAI-compat is returned unchanged", + providerType: store.ProviderOpenAICompat, + apiBase: "http://host:11434", + want: "http://host:11434", + }, + { + name: "Anthropic native is returned unchanged", + providerType: store.ProviderAnthropicNative, + apiBase: "https://api.anthropic.com", + want: "https://api.anthropic.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &ProvidersHandler{} + p := &store.LLMProviderData{ + ProviderType: tt.providerType, + APIBase: tt.apiBase, + } + got := h.resolveAPIBase(p) + if got != tt.want { + t.Fatalf("resolveAPIBase() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestResolveAPIBaseUsesAPIBaseFallback checks the config/env fallback path: when +// the DB value is empty and a fallback is configured, the fallback is used — and +// the Ollama safety-net normalization is applied to fallback values too. +func TestResolveAPIBaseUsesAPIBaseFallback(t *testing.T) { + tests := []struct { + name string + providerType string + storedBase string + fallback string + want string + }{ + { + name: "empty DB, non-Ollama fallback returned as-is", + providerType: store.ProviderOpenAICompat, + storedBase: "", + fallback: "https://api.example.com/v1", + want: "https://api.example.com/v1", + }, + { + name: "empty DB, Ollama fallback missing /v1 gets safety net", + providerType: store.ProviderOllama, + storedBase: "", + fallback: "http://localhost:11434", + want: "http://localhost:11434/v1", + }, + { + name: "empty DB, Ollama fallback already has /v1 — no double", + providerType: store.ProviderOllama, + storedBase: "", + fallback: "http://localhost:11434/v1", + want: "http://localhost:11434/v1", + }, + { + name: "no fallback, empty DB, empty result", + providerType: store.ProviderOllama, + storedBase: "", + fallback: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &ProvidersHandler{ + apiBaseFallback: func(_ string) string { return tt.fallback }, + } + p := &store.LLMProviderData{ + ProviderType: tt.providerType, + APIBase: tt.storedBase, + } + got := h.resolveAPIBase(p) + if got != tt.want { + t.Fatalf("resolveAPIBase() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestRegisterInMemoryOllamaURLNormalization verifies that registerInMemory registers the +// Ollama provider with a correct base URL (already normalized with /v1 at write time) +// and does NOT produce a doubled /v1 in the registered provider. +func TestRegisterInMemoryOllamaURLNormalization(t *testing.T) { + tests := []struct { + name string + apiBase string // value as stored in DB (post-normalization) + wantURL string // expected URLin the registered provider + }{ + { + name: "normalized /v1 is registered unchanged", + apiBase: "http://host:11434/v1", + wantURL: "http://host:11434/v1", + }, + { + name: "empty api_base falls back to default with /v1", + apiBase: "", + wantURL: "http://localhost:11434/v1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + providerReg := providers.NewRegistry(nil) + handler := NewProvidersHandler(newMockProviderStore(), newMockSecretsStore(), providerReg, "") + + p := &store.LLMProviderData{ + BaseModel: store.BaseModel{ID: uuid.New()}, + TenantID: uuid.New(), + Name: "my-ollama", + ProviderType: store.ProviderOllama, + APIBase: tt.apiBase, + Enabled: true, + } + + handler.registerInMemory(p) + + runtimeProvider, err := providerReg.GetForTenant(p.TenantID, p.Name) + if err != nil { + t.Fatalf("GetForTenant() error = %v", err) + } + + type apiBaseProvider interface { + APIBase() string + } + abp, ok := runtimeProvider.(apiBaseProvider) + if !ok { + t.Fatalf("registered provider does not implement APIBase(), type = %T", runtimeProvider) + } + + got := abp.APIBase() + if got != tt.wantURL { + t.Fatalf("registered APIBase = %q, want %q", got, tt.wantURL) + } + }) + } +}