diff --git a/server/llm_test_fallback_test.go b/server/llm_test_fallback_test.go index 052b76e..54895df 100644 --- a/server/llm_test_fallback_test.go +++ b/server/llm_test_fallback_test.go @@ -92,3 +92,64 @@ func TestLLMTestFallbackMatchesRuntimeNormalization(t *testing.T) { t.Fatalf("ConfigFrom trims: %q", runtime.APIKey) } } + +// The stored-profile test must reach the profile's own endpoint: an empty form +// (the list-view test button) fills provider/model/base_url from the row, so +// the key lands on the configured host instead of the canonical default one. +func TestLLMTestStoredProfileUsesProfileEndpoint(t *testing.T) { + m, err := NewManager(t.TempDir(), "") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { m.Close() }) + if m.pg == nil { + t.Skip("no postgres for this run") + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir()) + + var hits atomic.Int32 + var gotKey atomic.Value + fake := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + gotKey.Store(r.Header.Get("x-api-key")) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"m","type":"message","role":"assistant","model":"m","content":[{"type":"text","text":"OK"}],"stop_reason":"end_turn","usage":{"input_tokens":1,"output_tokens":1}}`)) + })) + t.Cleanup(fake.Close) + + pid, err := m.pg.SaveProfile(&db.LLMProfile{ + Name: "stored-ep", Format: "anthropic", Model: "claude-x", + BaseURL: fake.URL, APIKey: "k1", + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = m.pg.Exec("DELETE FROM llm_profiles WHERE id=$1", pid) }) + + // Empty form on purpose: no provider, model, base_url, or streaming. + body, _ := json.Marshal(map[string]any{"profile_id": pid, "api_key": ""}) + req := httptest.NewRequest(http.MethodPost, "/api/llm/test", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + s.testLLM(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("handler status %d", rec.Code) + } + if hits.Load() != 1 { + t.Fatalf("stored-profile test must hit the profile endpoint exactly once, hits=%d", hits.Load()) + } + if k, _ := gotKey.Load().(string); k != "k1" { + t.Fatalf("test request must send the stored key, header was %q", k) + } + var resp struct { + OK bool `json:"ok"` + Error string `json:"error"` + } + _ = json.Unmarshal(rec.Body.Bytes(), &resp) + if !resp.OK { + t.Fatalf("stored-profile round-trip not ok: %s", resp.Error) + } +} diff --git a/server/server.go b/server/server.go index a2c2e34..35c3c1f 100644 --- a/server/server.go +++ b/server/server.go @@ -1385,6 +1385,41 @@ func (s *Server) testLLM(w http.ResponseWriter, r *http.Request) { writeErr(w, 400, err.Error()) return } + // A stored profile must be tested AS STORED: the list-view test button + // sends only profile_id, and ConfigFrom's defaults for an empty + // provider/base_url point at the canonical provider endpoint, sending this + // profile's key to the wrong host (the stored-key 401s). Fill whatever the + // form left empty from the row first; explicit form values still win. + var stored *db.LLMProfile + if req.ProfileID != nil { + if p, err := s.m.pg.ProfileByID(*req.ProfileID); err == nil && p != nil { + stored = p + } + } + if stored != nil { + if strings.TrimSpace(req.Provider) == "" { + req.Provider = stored.Format + } + if strings.TrimSpace(req.Model) == "" { + req.Model = stored.Model + } + if strings.TrimSpace(req.BaseURL) == "" { + req.BaseURL = stored.BaseURL + } + if strings.TrimSpace(req.Proxy) == "" { + req.Proxy = stored.Proxy + } + if req.Streaming == nil { + st := stored.Streaming + req.Streaming = &st + } + if strings.TrimSpace(req.ThinkingType) == "" { + req.ThinkingType = stored.ThinkingType + } + if strings.TrimSpace(req.ReasoningEffort) == "" { + req.ReasoningEffort = stored.ReasoningEffort + } + } cfg := agent.ConfigFrom(req.Provider, req.Model, req.BaseURL, req.APIKey, req.Proxy) // mirror production: send the SAME thinking params so a provider that rejects the // reasoning_effort/thinking field fails the test too (no false "test ok, run 400"). @@ -1402,17 +1437,15 @@ func (s *Server) testLLM(w http.ResponseWriter, r *http.Request) { // API key resolution precedence: form input > the named profile's stored key > the global config's key. // A stored profile's key is never sent back to the browser, so the form is empty when testing a stored profile and the key comes from the DB. // Same for the session header name: when the form omits it, the stored profile's value is the fallback. - if req.ProfileID != nil && (cfg.APIKey == "" || cfg.SessionHeaderKey == "") { - if p, err := s.m.pg.ProfileByID(*req.ProfileID); err == nil && p != nil { - // Trim like ConfigFrom does for form values: a key stored with - // surrounding whitespace runs fine in conversations (the runtime - // path re-trims) but would be sent raw here and fail auth. - if cfg.APIKey == "" { - cfg.APIKey = strings.TrimSpace(p.APIKey) - } - if cfg.SessionHeaderKey == "" { - cfg.SessionHeaderKey = strings.TrimSpace(p.SessionHeaderKey) - } + if stored != nil && (cfg.APIKey == "" || cfg.SessionHeaderKey == "") { + // Trim like ConfigFrom does for form values: a key stored with + // surrounding whitespace runs fine in conversations (the runtime + // path re-trims) but would be sent raw here and fail auth. + if cfg.APIKey == "" { + cfg.APIKey = strings.TrimSpace(stored.APIKey) + } + if cfg.SessionHeaderKey == "" { + cfg.SessionHeaderKey = strings.TrimSpace(stored.SessionHeaderKey) } } if cfg.APIKey == "" {