AI settings API with key management (#80)

Co-authored-by: dev_agent <dev_agent@cubecraftlabs.com>

internal/ai/aisettings/aisettings.go

@@ -0,0 +1,478 @@
+// Package aisettings reads and changes the household's AI settings — the
+// ai_settings row seeded by household.SeedAISettings — and builds the
+// provider and harness.Config they describe. The API key is sealed with the
+// secret box and never leaves the package except to the provider.
+package aisettings
+
+import (
+	"bytes"
+	"context"
+	"database/sql"
+	"errors"
+	"fmt"
+	"math"
+	"net/url"
+	"strings"
+	"unicode/utf8"
+
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm/presets"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secureurl"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
+)
+
+const (
+	// MaxSystemPromptChars is the longest system prompt accepted, in
+	// characters.
+	MaxSystemPromptChars = 8000
+	// MinKeyLength is the shortest API key accepted, so its last four
+	// characters never give away most of it.
+	MinKeyLength = 8
+)
+
+var (
+	// ErrNotConfigured is returned when the stored AI settings cannot
+	// build a provider, e.g. there is no API key yet. It is joined with
+	// the llm configuration error that says why.
+	ErrNotConfigured = errors.New("aisettings: AI provider is not configured")
+	// ErrProviderFailed is returned when a call to the provider fails. It
+	// is joined with the provider's error, so the llm typed error it
+	// matches says why.
+	ErrProviderFailed = errors.New("aisettings: AI provider call failed")
+	// ErrChanged is returned when the AI settings changed while
+	// their key was being verified, so the result applies to old settings.
+	ErrChanged = errors.New("aisettings: AI settings changed during verification")
+)
+
+// Settings is the household's AI configuration as the API shows it. The
+// API key is never part of it: only its last four characters.
+type Settings struct {
+	Provider    string  `json:"provider"`
+	BaseURL     string  `json:"base_url"`
+	Model       string  `json:"model"`
+	Temperature float64 `json:"temperature"`
+	// SystemPrompt is the admin persona, nil when none is stored.
+	SystemPrompt *string `json:"system_prompt"`
+	// APIKeyLast4 is nil when no key is stored.
+	APIKeyLast4 *string `json:"api_key_last4"`
+	// KeyVerifiedAt is when the stored key was last verified with these
+	// provider settings, nil when it has not been.
+	KeyVerifiedAt *string `json:"key_verified_at"`
+	UpdatedAt     string  `json:"updated_at"`
+}
+
+// Patch changes the settings. A nil field is left as it is.
+type Patch struct {
+	Provider     *string  `json:"provider"`
+	BaseURL      *string  `json:"base_url"`
+	Model        *string  `json:"model"`
+	Temperature  *float64 `json:"temperature"`
+	SystemPrompt *string  `json:"system_prompt"`
+}
+
+// Model is one model the provider offers.
+type Model struct {
+	ID          string `json:"id"`
+	DisplayName string `json:"display_name"`
+}
+
+// Problem is one invalid field, named by its JSON key.
+type Problem struct {
+	Field  string
+	Reason string
+}
+
+// ValidationError lists every problem with an Patch or API
+// key, or with the settings it would leave. No problem quotes the key.
+type ValidationError struct {
+	Problems []Problem
+}
+
+func (e *ValidationError) Error() string {
+	parts := make([]string, len(e.Problems))
+	for i, p := range e.Problems {
+		parts[i] = p.Field + ": " + p.Reason
+	}
+	return "invalid AI settings: " + strings.Join(parts, "; ")
+}
+
+// Service reads and changes the ai_settings row and builds the provider it
+// names. API keys are sealed with its secret box and never leave it except
+// to the provider.
+type Service struct {
+	st        *store.Store
+	box       *secretbox.Box
+	publicURL string
+	// newProvider and newAccount are presets.FromSettings and
+	// presets.ForAccount; tests substitute a fake provider.
+	newProvider harness.NewProvider
+	newAccount  func(llm.Settings, string) (presets.Account, error)
+}
+
+// New returns the service over st, sealing keys with box.
+// publicURL is MEALQUEST_PUBLIC_URL, which some presets send.
+func New(st *store.Store, box *secretbox.Box, publicURL string) (*Service, error) {
+	switch {
+	case st == nil:
+		return nil, errors.New("aisettings: nil store")
+	case box == nil:
+		return nil, errors.New("aisettings: nil secret box")
+	case publicURL == "":
+		return nil, errors.New("aisettings: empty public URL")
+	}
+	return &Service{st: st, box: box, publicURL: publicURL, newProvider: presets.FromSettings, newAccount: presets.ForAccount}, nil
+}
+
+// Get returns the settings.
+func (s *Service) Get(ctx context.Context) (Settings, error) {
+	row, err := s.read(ctx, db.New(s.st.Reader))
+	if err != nil {
+		return Settings{}, err
+	}
+	return settingsFrom(row), nil
+}
+
+// Update applies patch and returns the settings it leaves. The provider
+// and base URL cannot change while an API key is stored, so the key never
+// reaches an endpoint it was not stored for. Every problem is reported in
+// a *ValidationError and nothing changes.
+func (s *Service) Update(ctx context.Context, patch Patch) (Settings, error) {
+	return s.change(ctx, func(row db.AiSetting) (db.AiSetting, error) {
+		return applyPatch(row, patch)
+	})
+}
+
+// SetAPIKey seals key and records its last four characters, clearing
+// key_verified_at. An empty key removes the stored one. A preset without
+// a fixed base URL takes a key only when its base URL is secure (see
+// secureurl.Check).
+func (s *Service) SetAPIKey(ctx context.Context, key string) (Settings, error) {
+	return s.change(ctx, func(row db.AiSetting) (db.AiSetting, error) {
+		row.ApiKeyCt, row.ApiKeyLast4, row.KeyVerifiedAt = nil, sql.NullString{}, sql.NullString{}
+		if key == "" {
+			return row, nil
+		}
+		p, _ := presets.Lookup(row.Provider)
+		var reason string
+		switch {
+		case p.Key == presets.KeyNone:
+			reason = row.Provider + " takes no API key"
+		case p.BaseURL == "" && !secureBaseURL(row.BaseUrl):
+			reason = "is sent only over https://, or http:// to localhost: set such a base_url first"
+		case !validHeaderValue(key):
+			reason = "has a character not allowed in a header"
+		case len(key) < MinKeyLength:
+			reason = fmt.Sprintf("must be at least %d characters", MinKeyLength)
+		}
+		if reason != "" {
+			return db.AiSetting{}, &ValidationError{Problems: []Problem{{Field: "api_key", Reason: reason}}}
+		}
+		ct, err := s.box.Encrypt([]byte(key))
+		if err != nil {
+			return db.AiSetting{}, fmt.Errorf("aisettings: seal AI API key: %w", err)
+		}
+		row.ApiKeyCt = ct
+		row.ApiKeyLast4 = sql.NullString{String: key[len(key)-4:], Valid: true}
+		return row, nil
+	})
+}
+
+// ResetPrompt restores household.DefaultPersonaPrompt as the system prompt.
+func (s *Service) ResetPrompt(ctx context.Context) (Settings, error) {
+	return s.change(ctx, func(row db.AiSetting) (db.AiSetting, error) {
+		row.SystemPrompt = sql.NullString{String: household.DefaultPersonaPrompt, Valid: true}
+		return row, nil
+	})
+}
+
+// Verify checks the stored key with the provider and, when it is
+// accepted, records key_verified_at. Settings that cannot build a
+// provider fail with ErrNotConfigured; a provider failure matches
+// ErrProviderFailed and the llm typed error the provider returned. If the
+// settings change while the provider is asked, nothing is recorded and the
+// error is ErrChanged.
+func (s *Service) Verify(ctx context.Context) (Settings, error) {
+	row, err := s.read(ctx, db.New(s.st.Reader))
+	if err != nil {
+		return Settings{}, err
+	}
+	acct, key, err := s.account(row)
+	if err != nil {
+		return Settings{}, err
+	}
+	if err := acct.Verify(ctx); err != nil {
+		return Settings{}, redact(fmt.Errorf("%w: verify: %w", ErrProviderFailed, err), key)
+	}
+	verified := store.Now().String()
+	return s.change(ctx, func(now db.AiSetting) (db.AiSetting, error) {
+		if now.Provider != row.Provider || now.BaseUrl != row.BaseUrl || !bytes.Equal(now.ApiKeyCt, row.ApiKeyCt) {
+			return db.AiSetting{}, ErrChanged
+		}
+		now.KeyVerifiedAt = sql.NullString{String: verified, Valid: true}
+		return now, nil
+	})
+}
+
+// Models returns the provider's live model list with the API key removed
+// from every model's ID and display name. Its errors are Verify's.
+func (s *Service) Models(ctx context.Context) ([]Model, error) {
+	row, err := s.read(ctx, db.New(s.st.Reader))
+	if err != nil {
+		return nil, err
+	}
+	acct, key, err := s.account(row)
+	if err != nil {
+		return nil, err
+	}
+	models, err := acct.ListModels(ctx)
+	if err != nil {
+		return nil, redact(fmt.Errorf("%w: list models: %w", ErrProviderFailed, err), key)
+	}
+	out := make([]Model, len(models))
+	for i, m := range models {
+		out[i] = Model{ID: llm.Redact(m.ID, key), DisplayName: llm.Redact(m.DisplayName, key)}
+	}
+	return out, nil
+}
+
+// HarnessConfig returns the harness.Config for a skill run with the
+// current settings, sharing guard. Read it for each run, so a run always
+// uses the latest saved provider, model, temperature and persona.
+func (s *Service) HarnessConfig(ctx context.Context, guard *harness.Guard) (harness.Config, error) {
+	row, err := s.read(ctx, db.New(s.st.Reader))
+	if err != nil {
+		return harness.Config{}, err
+	}
+	key, err := s.key(row)
+	if err != nil {
+		return harness.Config{}, err
+	}
+	return harness.Config{
+		Settings:    s.llmSettings(row),
+		APIKey:      key,
+		NewProvider: s.newProvider,
+		Persona:     row.SystemPrompt.String,
+		Store:       s.st,
+		Guard:       guard,
+	}, nil
+}
+
+// account builds the provider's Account from row, returning the key too
+// so the caller can redact it.
+func (s *Service) account(row db.AiSetting) (presets.Account, string, error) {
+	key, err := s.key(row)
+	if err != nil {
+		return nil, "", err
+	}
+	acct, err := s.newAccount(s.llmSettings(row), key)
+	if err != nil {
+		return nil, "", redact(fmt.Errorf("%w: %w", ErrNotConfigured, err), key)
+	}
+	return acct, key, nil
+}
+
+func (s *Service) llmSettings(row db.AiSetting) llm.Settings {
+	return llm.Settings{
+		Provider:    row.Provider,
+		BaseURL:     row.BaseUrl,
+		Model:       row.Model,
+		Temperature: row.Temperature,
+		PublicURL:   s.publicURL,
+	}
+}
+
+// key opens row's API key; "" when none is stored.
+func (s *Service) key(row db.AiSetting) (string, error) {
+	if len(row.ApiKeyCt) == 0 {
+		return "", nil
+	}
+	key, err := s.box.Decrypt(row.ApiKeyCt)
+	if err != nil {
+		return "", fmt.Errorf("aisettings: open AI API key: %w", err)
+	}
+	return string(key), nil
+}
+
+func (s *Service) read(ctx context.Context, q *db.Queries) (db.AiSetting, error) {
+	row, err := q.GetAISettings(ctx)
+	if errors.Is(err, sql.ErrNoRows) {
+		return db.AiSetting{}, errors.New("aisettings: AI settings are not seeded")
+	}
+	if err != nil {
+		return db.AiSetting{}, fmt.Errorf("aisettings: read AI settings: %w", err)
+	}
+	return row, nil
+}
+
+// change saves edit's result for the current row in one transaction.
+func (s *Service) change(ctx context.Context, edit func(db.AiSetting) (db.AiSetting, error)) (Settings, error) {
+	var saved db.AiSetting
+	err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
+		q := db.New(tx)
+		row, err := s.read(ctx, q)
+		if err != nil {
+			return err
+		}
+		next, err := edit(row)
+		if err != nil {
+			return err
+		}
+		saved, err = q.UpsertAISettings(ctx, db.UpsertAISettingsParams{
+			Provider:      next.Provider,
+			BaseUrl:       next.BaseUrl,
+			Model:         next.Model,
+			Temperature:   next.Temperature,
+			SystemPrompt:  next.SystemPrompt,
+			ApiKeyCt:      next.ApiKeyCt,
+			ApiKeyLast4:   next.ApiKeyLast4,
+			KeyVerifiedAt: next.KeyVerifiedAt,
+			CreatedAt:     row.CreatedAt,
+			UpdatedAt:     store.Now().String(),
+		})
+		if err != nil {
+			return fmt.Errorf("aisettings: save AI settings: %w", err)
+		}
+		return nil
+	})
+	if err != nil {
+		return Settings{}, err
+	}
+	return settingsFrom(saved), nil
+}
+
+// applyPatch returns row with patch applied, validated.
+func applyPatch(row db.AiSetting, patch Patch) (db.AiSetting, error) {
+	var problems []Problem
+	fail := func(field, reason string) {
+		problems = append(problems, Problem{Field: field, Reason: reason})
+	}
+	if patch == (Patch{}) {
+		fail("settings", "no settings to change")
+	}
+	before := row
+	if patch.Provider != nil {
+		if _, ok := presets.Lookup(*patch.Provider); ok {
+			row.Provider = *patch.Provider
+		} else {
+			fail("provider", "must be one of "+strings.Join(presets.Names(), ", "))
+		}
+	}
+	if patch.BaseURL != nil {
+		row.BaseUrl = *patch.BaseURL
+	}
+	if patch.Model != nil {
+		switch m := *patch.Model; {
+		case strings.TrimSpace(m) == "":
+			fail("model", "must not be empty")
+		case !validHeaderValue(m):
+			fail("model", "must not contain control characters")
+		default:
+			row.Model = m
+		}
+	}
+	if patch.Temperature != nil {
+		t := *patch.Temperature
+		tenths := math.Round(t * 10)
+		if t < 0 || t > 1 || math.Abs(t*10-tenths) > 1e-9 {
+			fail("temperature", "must be between 0 and 1 in steps of 0.1")
+		} else {
+			row.Temperature = tenths / 10
+		}
+	}
+	if patch.SystemPrompt != nil {
+		if n := utf8.RuneCountInString(*patch.SystemPrompt); n > MaxSystemPromptChars {
+			fail("system_prompt", fmt.Sprintf("must be at most %d characters, got %d", MaxSystemPromptChars, n))
+		} else {
+			row.SystemPrompt = sql.NullString{String: *patch.SystemPrompt, Valid: true}
+		}
+	}
+	if p, ok := presets.Lookup(row.Provider); ok && len(problems) == 0 {
+		switch {
+		case p.BaseURL != "" && row.BaseUrl != "" && row.BaseUrl != p.BaseURL:
+			fail("base_url", fmt.Sprintf("is fixed for %s: must be empty or %s", p.Name, p.BaseURL))
+		case p.BaseURL == "" && row.BaseUrl != "":
+			// The preset's own check of the URL; its errors never quote it.
+			if _, err := presets.ForAccount(llm.Settings{Provider: p.Name, BaseURL: row.BaseUrl}, ""); err != nil {
+				fail("base_url", err.Error())
+			}
+		}
+	}
+	// The stored key is only ever sent where it was stored for: moving it
+	// to another provider or server would let a changed base URL receive
+	// it. Remove the key, change the endpoint, then store the new key.
+	if len(problems) == 0 && len(row.ApiKeyCt) > 0 {
+		if row.Provider != before.Provider {
+			fail("provider", "cannot change while an API key is stored: remove the key first")
+		}
+		if row.BaseUrl != before.BaseUrl {
+			fail("base_url", "cannot change while an API key is stored: remove the key first")
+		}
+	}
+	if len(problems) > 0 {
+		return db.AiSetting{}, &ValidationError{Problems: problems}
+	}
+	return row, nil
+}
+
+func settingsFrom(row db.AiSetting) Settings {
+	return Settings{
+		Provider:      row.Provider,
+		BaseURL:       row.BaseUrl,
+		Model:         row.Model,
+		Temperature:   row.Temperature,
+		SystemPrompt:  nullString(row.SystemPrompt),
+		APIKeyLast4:   nullString(row.ApiKeyLast4),
+		KeyVerifiedAt: nullString(row.KeyVerifiedAt),
+		UpdatedAt:     row.UpdatedAt,
+	}
+}
+
+func nullString(s sql.NullString) *string {
+	if !s.Valid {
+		return nil
+	}
+	return &s.String
+}
+
+// secureBaseURL reports whether base is a URL a key may be sent to.
+func secureBaseURL(base string) bool {
+	u, err := url.Parse(base)
+	return base != "" && err == nil && secureurl.Check(u) == nil
+}
+
+// validHeaderValue reports whether value has no control character other
+// than tab.
+func validHeaderValue(value string) bool {
+	for i := 0; i < len(value); i++ {
+		if c := value[i]; (c < 0x20 && c != '\t') || c == 0x7f {
+			return false
+		}
+	}
+	return true
+}
+
+// redactedError is an error with the API key removed from its text. It
+// does not unwrap, because the original may hold the key; Is still matches
+// the sentinels the original wraps.
+type redactedError struct {
+	text string
+	err  error
+}
+
+func (e *redactedError) Error() string { return e.text }
+
+// Is reports whether the original error matches target.
+func (e *redactedError) Is(target error) bool { return errors.Is(e.err, target) }
+
+// redact returns err with key removed from its text; an empty key returns
+// err unchanged.
+func redact(err error, key string) error {
+	if key == "" {
+		return err
+	}
+	return &redactedError{text: llm.Redact(err.Error(), key), err: err}
+}

internal/ai/aisettings/aisettings_test.go

@@ -0,0 +1,463 @@
+package aisettings
+
+import (
+	"context"
+	"crypto/rand"
+	"encoding/base64"
+	"encoding/json"
+	"errors"
+	"reflect"
+	"slices"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm/presets"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations"
+)
+
+const testKey = "sk-ant-test-secret-key-41ab"
+
+// newService returns a Service over a migrated store with the seed AI
+// settings.
+func newService(t *testing.T) (*Service, *store.Store) {
+	t.Helper()
+	ctx := context.Background()
+	st, err := store.Open(ctx, t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		if err := st.Close(); err != nil {
+			t.Error(err)
+		}
+	})
+	if err := store.Migrate(ctx, st.Writer, migrations.FS); err != nil {
+		t.Fatal(err)
+	}
+	if err := household.SeedAISettings(ctx, st); err != nil {
+		t.Fatal(err)
+	}
+	raw := make([]byte, secretbox.KeySize)
+	rand.Read(raw)
+	box, err := secretbox.New(base64.StdEncoding.EncodeToString(raw))
+	if err != nil {
+		t.Fatal(err)
+	}
+	s, err := New(st, box, "https://mealquest.example")
+	if err != nil {
+		t.Fatal(err)
+	}
+	return s, st
+}
+
+// useFake makes s build f for every run, verification and model list, and
+// returns a func reporting the settings and key each build was given.
+func useFake(s *Service, f *llm.Fake) func() ([]llm.Settings, []string) {
+	var mu sync.Mutex
+	var settings []llm.Settings
+	var keys []string
+	record := func(set llm.Settings, key string) {
+		mu.Lock()
+		defer mu.Unlock()
+		settings = append(settings, set)
+		keys = append(keys, key)
+	}
+	s.newProvider = func(set llm.Settings, key string) (llm.Provider, error) {
+		record(set, key)
+		return f, nil
+	}
+	s.newAccount = func(set llm.Settings, key string) (presets.Account, error) {
+		record(set, key)
+		return f, nil
+	}
+	return func() ([]llm.Settings, []string) {
+		mu.Lock()
+		defer mu.Unlock()
+		return settings, keys
+	}
+}
+
+func ptr[T any](v T) *T { return &v }
+
+// echoSkill asks the model to repeat a word; its prompt is the harness's
+// test template prompts/echo.v1.tmpl.
+type echoSkill struct{}
+
+type echoInput struct{ Word string }
+
+type echoOutput struct {
+	Echo string `json:"echo"`
+}
+
+func (echoSkill) Name() string { return "echo" }
+func (echoSkill) Version() int { return 1 }
+func (echoSkill) PromptInput(in echoInput) (harness.PromptInput, error) {
+	return harness.PromptInput{Data: in}, nil
+}
+func (echoSkill) Schema() llm.JSONSchema {
+	return llm.JSONSchema{Name: "echo", Schema: json.RawMessage(
+		`{"type":"object","properties":{"echo":{"type":"string"}},"required":["echo"],"additionalProperties":false}`)}
+}
+func (echoSkill) Validate(echoOutput) []string                   { return nil }
+func (echoSkill) PostProcess(out echoOutput) (echoOutput, error) { return out, nil }
+func (echoSkill) Timeout() time.Duration                         { return harness.DefaultTimeout }
+
+// A PATCHed model and temperature are what the next skill run uses: the
+// provider is built with them, the request carries the temperature and the
+// ai_runs row records both. A second PATCH changes the run after it.
+func TestPatchedModelAndTemperatureUsedByNextRun(t *testing.T) {
+	ctx := context.Background()
+	s, st := newService(t)
+	f := &llm.Fake{}
+	built := useFake(s, f)
+	guard, err := harness.NewGuard(harness.DefaultMaxCalls)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.SetAPIKey(ctx, testKey); err != nil {
+		t.Fatal(err)
+	}
+
+	for i, want := range []struct {
+		model string
+		temp  float64
+	}{{"claude-patched", 0.3}, {"claude-repatched", 1}} {
+		if _, err := s.Update(ctx, Patch{Model: ptr(want.model), Temperature: ptr(want.temp)}); err != nil {
+			t.Fatal(err)
+		}
+		cfg, err := s.HarnessConfig(ctx, guard)
+		if err != nil {
+			t.Fatal(err)
+		}
+		f.QueueGenerate(llm.GenerateResult{Response: llm.Response{JSON: json.RawMessage(`{"echo":"hi"}`)}})
+		if _, err := harness.Run(ctx, cfg, echoSkill{}, "test", echoInput{Word: "hi"}); err != nil {
+			t.Fatalf("run %d: %v", i, err)
+		}
+
+		settings, keys := built()
+		if got := settings[i]; got.Provider != "anthropic" || got.Model != want.model || got.Temperature != want.temp || keys[i] != testKey {
+			t.Errorf("run %d provider built with %+v, key ok %v; want model %s at %v", i, got, keys[i] == testKey, want.model, want.temp)
+		}
+		req := f.Requests()[i]
+		if req.Temperature == nil || *req.Temperature != want.temp {
+			t.Errorf("run %d request temperature = %v, want %v", i, req.Temperature, want.temp)
+		}
+		if !strings.HasPrefix(req.System, household.DefaultPersonaPrompt[:40]) {
+			t.Errorf("run %d system prompt does not start with the persona: %.80q", i, req.System)
+		}
+		var model string
+		var temp float64
+		err = st.Reader.QueryRowContext(ctx, `SELECT model, temperature FROM ai_runs ORDER BY created_at DESC, rowid DESC LIMIT 1`).Scan(&model, &temp)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if model != want.model || temp != want.temp {
+			t.Errorf("run %d ai_runs row = %s at %v, want %s at %v", i, model, temp, want.model, want.temp)
+		}
+	}
+}
+
+func TestUpdateValidates(t *testing.T) {
+	ctx := context.Background()
+	for _, c := range []struct {
+		name  string
+		patch Patch
+		field string
+	}{
+		{"empty", Patch{}, "settings"},
+		{"unknown provider", Patch{Provider: ptr("gemini")}, "provider"},
+		{"temperature above 1", Patch{Temperature: ptr(1.1)}, "temperature"},
+		{"negative temperature", Patch{Temperature: ptr(-0.1)}, "temperature"},
+		{"temperature between steps", Patch{Temperature: ptr(0.35)}, "temperature"},
+		{"empty model", Patch{Model: ptr(" ")}, "model"},
+		{"long system prompt", Patch{SystemPrompt: ptr(strings.Repeat("ü", MaxSystemPromptChars+1))}, "system_prompt"},
+		{"other base URL for a fixed preset", Patch{BaseURL: ptr("https://api.example/v1")}, "base_url"},
+		{"bad base URL", Patch{Provider: ptr("ollama"), BaseURL: ptr("ftp://ollama:11434")}, "base_url"},
+	} {
+		t.Run(c.name, func(t *testing.T) {
+			s, _ := newService(t)
+			before, err := s.Get(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+			_, err = s.Update(ctx, c.patch)
+			var invalid *ValidationError
+			if !errors.As(err, &invalid) || len(invalid.Problems) != 1 || invalid.Problems[0].Field != c.field {
+				t.Fatalf("Update = %v, want one %s problem", err, c.field)
+			}
+			if after, err := s.Get(ctx); err != nil || !reflect.DeepEqual(after, before) {
+				t.Errorf("settings changed by a refused patch: %+v, %v", after, err)
+			}
+		})
+	}
+}
+
+func TestUpdateAccepts(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	for _, temp := range []float64{0, 0.1, 0.2, 0.3, 0.6, 0.7, 0.9, 1} {
+		got, err := s.Update(ctx, Patch{Temperature: ptr(temp)})
+		if err != nil || got.Temperature != temp {
+			t.Errorf("temperature %v = %v, %v", temp, got.Temperature, err)
+		}
+	}
+	prompt := strings.Repeat("ü", MaxSystemPromptChars)
+	if got, err := s.Update(ctx, Patch{SystemPrompt: &prompt}); err != nil || *got.SystemPrompt != prompt {
+		t.Errorf("system prompt of %d characters: %v", MaxSystemPromptChars, err)
+	}
+	for _, p := range presets.Names() {
+		patch := Patch{Provider: ptr(p), BaseURL: ptr("")}
+		if pr, _ := presets.Lookup(p); pr.BaseURL == "" {
+			patch.BaseURL = ptr("http://llm.local:11434")
+		}
+		if got, err := s.Update(ctx, patch); err != nil || got.Provider != p {
+			t.Errorf("provider %s: %+v, %v", p, got, err)
+		}
+	}
+}
+
+// While a key is stored the provider and base URL cannot change, so the
+// key never reaches an endpoint it was not stored for; the model,
+// temperature and system prompt can, keeping the verification.
+func TestEndpointLockedWhileKeyStored(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	f := &llm.Fake{}
+	useFake(s, f)
+	if _, err := s.Update(ctx, Patch{Provider: ptr("openai_compatible"), BaseURL: ptr("https://llm.example/v1")}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.SetAPIKey(ctx, testKey); err != nil {
+		t.Fatal(err)
+	}
+	f.QueueVerify(nil)
+	if got, err := s.Verify(ctx); err != nil || got.KeyVerifiedAt == nil {
+		t.Fatalf("Verify = %+v, %v", got, err)
+	}
+	got, err := s.Update(ctx, Patch{Model: ptr("m"), Temperature: ptr(0.5), SystemPrompt: ptr("p")})
+	if err != nil || got.KeyVerifiedAt == nil {
+		t.Errorf("model change = %+v, %v; want verification kept", got, err)
+	}
+	before, err := s.Get(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, c := range []struct {
+		patch Patch
+		field string
+	}{
+		{Patch{BaseURL: ptr("https://attacker.example/v1")}, "base_url"},
+		{Patch{Provider: ptr("openai"), BaseURL: ptr("")}, "provider"},
+		{Patch{Provider: ptr("ollama"), BaseURL: ptr("http://localhost:11434")}, "provider"},
+	} {
+		var invalid *ValidationError
+		_, err := s.Update(ctx, c.patch)
+		if !errors.As(err, &invalid) || !slices.ContainsFunc(invalid.Problems, func(p Problem) bool { return p.Field == c.field }) {
+			t.Errorf("Update(%+v) with a key stored = %v, want a %s problem", c.patch, err, c.field)
+		}
+	}
+	if after, err := s.Get(ctx); err != nil || !reflect.DeepEqual(after, before) {
+		t.Errorf("refused endpoint change altered the settings: %+v, %v", after, err)
+	}
+
+	// Removing the key frees the endpoint; a new key is unverified.
+	if _, err := s.SetAPIKey(ctx, ""); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Update(ctx, Patch{Provider: ptr("openai"), BaseURL: ptr("")}); err != nil {
+		t.Fatal(err)
+	}
+	got, err = s.SetAPIKey(ctx, "sk-another-key-9999")
+	if err != nil || got.KeyVerifiedAt != nil || *got.APIKeyLast4 != "9999" {
+		t.Errorf("new key = %+v, %v; want it unverified", got, err)
+	}
+}
+
+// A key is never stored for a base URL it would cross the network to in
+// plain text.
+func TestSetAPIKeyNeedsSecureBaseURL(t *testing.T) {
+	ctx := context.Background()
+	for _, c := range []struct {
+		base string
+		ok   bool
+	}{
+		{"", false},
+		{"http://llm.lan:8080/v1", false},
+		{"http://127.0.0.1:8080/v1", true},
+		{"https://llm.example/v1", true},
+	} {
+		s, _ := newService(t)
+		patch := Patch{Provider: ptr("openai_compatible")}
+		if c.base != "" {
+			patch.BaseURL = ptr(c.base)
+		}
+		if _, err := s.Update(ctx, patch); err != nil {
+			t.Fatal(err)
+		}
+		_, err := s.SetAPIKey(ctx, testKey)
+		var invalid *ValidationError
+		if c.ok != (err == nil) || (!c.ok && !errors.As(err, &invalid)) {
+			t.Errorf("SetAPIKey with base URL %q = %v, want ok %v", c.base, err, c.ok)
+		}
+	}
+}
+
+func TestVerifyReturnsProviderTypedError(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	f := &llm.Fake{}
+	useFake(s, f)
+	if _, err := s.SetAPIKey(ctx, testKey); err != nil {
+		t.Fatal(err)
+	}
+	for _, want := range []error{llm.ErrAuth, llm.NewRateLimitedError(time.Second, testKey), llm.NewProviderError(500, []byte("bad "+testKey), testKey)} {
+		f.QueueVerify(want)
+		_, err := s.Verify(ctx)
+		if !errors.Is(err, ErrProviderFailed) || !errors.Is(err, want) {
+			t.Errorf("Verify = %v, want ErrProviderFailed and %v", err, want)
+		}
+		if err != nil && strings.Contains(err.Error(), testKey) {
+			t.Errorf("Verify error holds the key: %v", err)
+		}
+	}
+	if got, err := s.Get(ctx); err != nil || got.KeyVerifiedAt != nil {
+		t.Errorf("failed verification recorded: %+v, %v", got, err)
+	}
+}
+
+// A key error that quotes the key, as a misbehaving adapter might, is
+// redacted by the service.
+func TestVerifyRedactsKey(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	f := &llm.Fake{}
+	useFake(s, f)
+	if _, err := s.SetAPIKey(ctx, testKey); err != nil {
+		t.Fatal(err)
+	}
+	f.QueueVerify(errors.Join(llm.ErrAuth, errors.New("rejected "+testKey)))
+	_, err := s.Verify(ctx)
+	if !errors.Is(err, llm.ErrAuth) || strings.Contains(err.Error(), testKey) {
+		t.Errorf("Verify = %v, want ErrAuth without the key", err)
+	}
+	f.QueueListModels(llm.ListModelsResult{Err: errors.New("rejected " + testKey)})
+	if _, err := s.Models(ctx); !errors.Is(err, ErrProviderFailed) || strings.Contains(err.Error(), testKey) {
+		t.Errorf("Models = %v, want ErrProviderFailed without the key", err)
+	}
+	f.QueueListModels(llm.ListModelsResult{Models: []llm.Model{{ID: "m-" + testKey, DisplayName: "Model " + testKey}}})
+	models, err := s.Models(ctx)
+	if err != nil || len(models) != 1 || models[0] != (Model{ID: "m-[REDACTED]", DisplayName: "Model [REDACTED]"}) {
+		t.Errorf("Models = %+v, %v, want the key redacted", models, err)
+	}
+}
+
+// Settings changed while the provider verifies the key are not marked
+// verified.
+func TestVerifyRefusesChangedSettings(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	if _, err := s.SetAPIKey(ctx, testKey); err != nil {
+		t.Fatal(err)
+	}
+	s.newAccount = func(llm.Settings, string) (presets.Account, error) {
+		return changingAccount{s: s}, nil
+	}
+	if _, err := s.Verify(ctx); !errors.Is(err, ErrChanged) {
+		t.Fatalf("Verify = %v, want ErrChanged", err)
+	}
+	if got, err := s.Get(ctx); err != nil || got.KeyVerifiedAt != nil || *got.APIKeyLast4 != "5678" {
+		t.Errorf("settings after a changed verification = %+v, %v", got, err)
+	}
+}
+
+// changingAccount replaces the stored key while it verifies.
+type changingAccount struct{ s *Service }
+
+func (a changingAccount) Verify(ctx context.Context) error {
+	_, err := a.s.SetAPIKey(ctx, "sk-replaced-5678")
+	return err
+}
+
+func (changingAccount) ListModels(context.Context) ([]llm.Model, error) { return nil, nil }
+
+func TestNotConfigured(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	// The seed is anthropic with no key; the real presets refuse it.
+	if _, err := s.Verify(ctx); !errors.Is(err, ErrNotConfigured) || !errors.Is(err, llm.ErrMissingKey) {
+		t.Errorf("Verify = %v, want ErrNotConfigured and ErrMissingKey", err)
+	}
+	if _, err := s.Models(ctx); !errors.Is(err, ErrNotConfigured) {
+		t.Errorf("Models = %v, want ErrNotConfigured", err)
+	}
+}
+
+func TestSetAPIKey(t *testing.T) {
+	ctx := context.Background()
+	s, st := newService(t)
+	got, err := s.SetAPIKey(ctx, testKey)
+	if err != nil || got.APIKeyLast4 == nil || *got.APIKeyLast4 != "41ab" {
+		t.Fatalf("SetAPIKey = %+v, %v", got, err)
+	}
+	var ct []byte
+	if err := st.Reader.QueryRowContext(ctx, `SELECT api_key_ct FROM ai_settings`).Scan(&ct); err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(ct), testKey) {
+		t.Error("the key is stored in plain text")
+	}
+	if key, err := s.key(mustRow(t, s)); err != nil || key != testKey {
+		t.Errorf("stored key opens to %q, %v", key, err)
+	}
+
+	for _, bad := range []string{"short", "sk-bad\r\nkey"} {
+		var invalid *ValidationError
+		if _, err := s.SetAPIKey(ctx, bad); !errors.As(err, &invalid) {
+			t.Errorf("SetAPIKey(%q) = %v, want a ValidationError", bad, err)
+		}
+	}
+
+	// Ollama takes no key: neither storing one nor switching to it with one.
+	var invalid *ValidationError
+	if _, err := s.Update(ctx, Patch{Provider: ptr("ollama"), BaseURL: ptr("http://ollama:11434")}); !errors.As(err, &invalid) {
+		t.Errorf("switch to ollama with a key = %v, want a ValidationError", err)
+	}
+	if got, err := s.SetAPIKey(ctx, ""); err != nil || got.APIKeyLast4 != nil {
+		t.Fatalf("remove key = %+v, %v", got, err)
+	}
+	if _, err := s.Update(ctx, Patch{Provider: ptr("ollama"), BaseURL: ptr("http://ollama:11434")}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.SetAPIKey(ctx, testKey); !errors.As(err, &invalid) {
+		t.Errorf("SetAPIKey for ollama = %v, want a ValidationError", err)
+	}
+}
+
+func TestResetPrompt(t *testing.T) {
+	ctx := context.Background()
+	s, _ := newService(t)
+	if _, err := s.Update(ctx, Patch{SystemPrompt: ptr("")}); err != nil {
+		t.Fatal(err)
+	}
+	got, err := s.ResetPrompt(ctx)
+	if err != nil || got.SystemPrompt == nil || *got.SystemPrompt != household.DefaultPersonaPrompt {
+		t.Errorf("ResetPrompt = %+v, %v", got, err)
+	}
+}
+
+func mustRow(t *testing.T, s *Service) db.AiSetting {
+	t.Helper()
+	row, err := s.read(context.Background(), db.New(s.st.Reader))
+	if err != nil {
+		t.Fatal(err)
+	}
+	return row
+}

internal/ai/llm/presets/presets.go

@@ -15,6 +15,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"math"
 	"regexp"
 	"slices"
@@ -115,6 +116,11 @@ func Lookup(name string) (Preset, bool) {
 	return p, ok
 }
 
+// Names returns every preset's name, sorted.
+func Names() []string {
+	return slices.Sorted(maps.Keys(all))
+}
+
 // all is every preset, by Settings.Provider name.
 var all = map[string]Preset{
 	"anthropic": {
@@ -181,19 +187,40 @@ var all = map[string]Preset{
 // preset's OmitTemperature matches. It is wrapped in llm.NewRetrying, so
 // Generate retries transient vendor failures and has the default timeout.
 func FromSettings(s llm.Settings, key string) (llm.Provider, error) {
-	cp, err := fromSettings(s, key)
+	cp, err := fromSettings(s, key, true)
 	if err != nil {
 		return nil, redact(err, key)
 	}
 	return llm.NewRetrying(cp), nil
 }
 
-func fromSettings(s llm.Settings, key string) (*configured, error) {
+// Account is what a provider offers before a model is chosen: the live
+// model list and a check of the credentials.
+type Account interface {
+	ListModels(ctx context.Context) ([]llm.Model, error)
+	Verify(ctx context.Context) error
+}
+
+// ForAccount returns the Account s describes, using key as its API key. It
+// checks s and key as FromSettings does except that s.Model may be empty,
+// so the household can verify its key and pick from the live model list
+// before it has chosen a model. Neither call is retried.
+func ForAccount(s llm.Settings, key string) (Account, error) {
+	cp, err := fromSettings(s, key, false)
+	if err != nil {
+		return nil, redact(err, key)
+	}
+	return cp, nil
+}
+
+// fromSettings builds the provider s describes; needModel is whether
+// s.Model is required.
+func fromSettings(s llm.Settings, key string, needModel bool) (*configured, error) {
 	p, ok := Lookup(s.Provider)
 	if !ok {
 		return nil, llm.ErrUnknownProvider
 	}
-	if err := check(p, s, key); err != nil {
+	if err := check(p, s, key, needModel); err != nil {
 		return nil, fmt.Errorf("%s: %w", p.Name, err)
 	}
 	base := p.BaseURL
@@ -233,8 +260,9 @@ func fromSettings(s llm.Settings, key string) (*configured, error) {
 	return cp, nil
 }
 
-// check validates s and key against p. Its errors never quote a value.
-func check(p Preset, s llm.Settings, key string) error {
+// check validates s and key against p, and that s names a model when
+// needModel is set. Its errors never quote a value.
+func check(p Preset, s llm.Settings, key string, needModel bool) error {
 	switch {
 	case p.Key == KeyRequired && key == "":
 		return llm.ErrMissingKey
@@ -249,7 +277,7 @@ func check(p Preset, s llm.Settings, key string) error {
 	case p.BaseURL != "" && s.BaseURL != "" && s.BaseURL != p.BaseURL:
 		return fmt.Errorf("%w: base URL is fixed by the preset", llm.ErrInvalidSetting)
 	}
-	if s.Model == "" {
+	if needModel && s.Model == "" {
 		return llm.ErrMissingModel
 	}
 	if p.RequiresPublicURL && s.PublicURL == "" {

internal/ai/llm/presets/presets_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"slices"
 	"strings"
 	"testing"
 
@@ -660,3 +661,35 @@ func TestFromSettingsRetriesTransportTimeout(t *testing.T) {
 		t.Errorf("sent %d requests, want 2", st.sent)
 	}
 }
+
+// ForAccount builds without a model, so a key can be verified and the
+// model list read before a model is chosen, but keeps every other check.
+func TestForAccount(t *testing.T) {
+	if _, err := ForAccount(llm.Settings{Provider: "openai"}, testKey); err != nil {
+		t.Errorf("ForAccount without a model = %v", err)
+	}
+	for _, c := range []struct {
+		s    llm.Settings
+		key  string
+		want error
+	}{
+		{llm.Settings{Provider: "gemini"}, testKey, llm.ErrUnknownProvider},
+		{llm.Settings{Provider: "openai"}, "", llm.ErrMissingKey},
+		{llm.Settings{Provider: "ollama"}, "", llm.ErrMissingBaseURL},
+		{llm.Settings{Provider: "openrouter"}, testKey, llm.ErrMissingPublicURL},
+	} {
+		if _, err := ForAccount(c.s, c.key); !errors.Is(err, c.want) {
+			t.Errorf("ForAccount(%s) = %v, want %v", c.s.Provider, err, c.want)
+		}
+	}
+	if _, err := FromSettings(llm.Settings{Provider: "openai"}, testKey); !errors.Is(err, llm.ErrMissingModel) {
+		t.Errorf("FromSettings without a model = %v, want ErrMissingModel", err)
+	}
+}
+
+func TestNames(t *testing.T) {
+	want := []string{"anthropic", "ollama", "openai", "openai_compatible", "openrouter", "xai"}
+	if got := Names(); !slices.Equal(got, want) {
+		t.Errorf("Names = %v, want %v", got, want)
+	}
+}