From eba1e505a5104810f81c5195e1617049616d2a9d Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 4 Oct 2026 09:31:49 -0400 Subject: [PATCH] Add xAI Grok provider preset Preset xai builds the OpenAI-compatible adapter against the fixed base URL https://api.x.ai/v1 with a required API key and no extra headers. It defaults to json_schema structured output and lets the household select json_object as a fallback; a request is never retried in another mode. Migration 0005_ai_provider_xai rebuilds ai_settings and ai_runs the 0004 way so their provider CHECK also accepts xai, keeping every 0004 value. Maestro task d8be638f-297f-4b7a-a545-74d193055904. Co-Authored-By: Claude Opus 5.5 --- internal/ai/llm/presets/presets.go | 9 +++ internal/ai/llm/presets/presets_test.go | 48 ++++++++++++ internal/ai/llm/settings.go | 2 +- internal/store/migrate_test.go | 98 ++++++++++++++++++++++++- internal/store/schema_test.go | 2 +- migrations/0005_ai_provider_xai.sql | 64 ++++++++++++++++ 6 files changed, 217 insertions(+), 6 deletions(-) create mode 100644 migrations/0005_ai_provider_xai.sql diff --git a/internal/ai/llm/presets/presets.go b/internal/ai/llm/presets/presets.go index d33d613..43b0d85 100644 --- a/internal/ai/llm/presets/presets.go +++ b/internal/ai/llm/presets/presets.go @@ -145,6 +145,15 @@ var all = map[string]Preset{ StructuredOutputs: []openai.StructuredOutput{openai.JSONSchema, openai.JSONObject}, OmitTemperature: []*regexp.Regexp{openRouterReasoningModels}, }, + "xai": { + Name: "xai", + Adapter: AdapterOpenAI, + BaseURL: "https://api.x.ai/v1", + Key: KeyRequired, + StructuredOutput: openai.JSONSchema, + FallbackStructuredOutput: openai.JSONObject, + StructuredOutputs: []openai.StructuredOutput{openai.JSONSchema, openai.JSONObject}, + }, "ollama": { Name: "ollama", Adapter: AdapterOllama, diff --git a/internal/ai/llm/presets/presets_test.go b/internal/ai/llm/presets/presets_test.go index 47f98c5..142c564 100644 --- a/internal/ai/llm/presets/presets_test.go +++ b/internal/ai/llm/presets/presets_test.go @@ -180,6 +180,43 @@ func TestOpenRouterFallbackModeIsJSONObject(t *testing.T) { } } +func TestXAIPreset(t *testing.T) { + c := generate(t, llm.Settings{Provider: "xai", Model: "grok-4", Temperature: 0.6}, testKey, schemaRequest()) + if c.url != "https://api.x.ai/v1/chat/completions" { + t.Fatalf("URL = %q", c.url) + } + if got := c.header.Get("Authorization"); got != "Bearer "+testKey { + t.Fatalf("Authorization = %q", got) + } + for _, h := range []string{"X-Api-Key", "HTTP-Referer", "X-Title"} { + if got := c.header.Get(h); got != "" { + t.Fatalf("%s = %q, want none", h, got) + } + } + if got := responseFormatType(t, c); got != "json_schema" { + t.Fatalf("default response_format type = %q, want json_schema", got) + } + if got, ok := temperature(t, c); !ok || got != 0.6 { + t.Fatalf("temperature = %v, %v; want 0.6", got, ok) + } +} + +func TestXAIFallbackModeIsJSONObject(t *testing.T) { + p, _ := Lookup("xai") + s := llm.Settings{Provider: "xai", Model: "grok-4", StructuredOutput: string(p.FallbackStructuredOutput)} + c := generate(t, s, testKey, schemaRequest()) + if got := responseFormatType(t, c); got != "json_object" { + t.Fatalf("response_format type = %q, want json_object", got) + } +} + +func TestXAIPresetAcceptsItsOwnBaseURL(t *testing.T) { + c := generate(t, llm.Settings{Provider: "xai", BaseURL: "https://api.x.ai/v1", Model: "grok-4"}, testKey, plainRequest()) + if c.url != "https://api.x.ai/v1/chat/completions" { + t.Fatalf("URL = %q", c.url) + } +} + func TestOllamaPreset(t *testing.T) { s := llm.Settings{Provider: "ollama", BaseURL: "http://ollama.lan:11434", Model: "llama3.2", Temperature: 0.5} c := generate(t, s, "", plainRequest()) @@ -318,6 +355,7 @@ func TestPresetTable(t *testing.T) { {"anthropic", AdapterAnthropic, "https://api.anthropic.com", KeyRequired}, {"openai", AdapterOpenAI, "https://api.openai.com/v1", KeyRequired}, {"openrouter", AdapterOpenAI, "https://openrouter.ai/api/v1", KeyRequired}, + {"xai", AdapterOpenAI, "https://api.x.ai/v1", KeyRequired}, {"ollama", AdapterOllama, "", KeyNone}, {"openai_compatible", AdapterOpenAI, "", KeyOptional}, } @@ -350,10 +388,12 @@ func TestConfigErrors(t *testing.T) { {"anthropic no key", llm.Settings{Provider: "anthropic", Model: "m"}, "", llm.ErrMissingKey}, {"openai no key", llm.Settings{Provider: "openai", Model: "m"}, "", llm.ErrMissingKey}, {"openrouter no key", llm.Settings{Provider: "openrouter", Model: "m", PublicURL: "https://x.example"}, "", llm.ErrMissingKey}, + {"xai no key", llm.Settings{Provider: "xai", Model: "grok-4"}, "", llm.ErrMissingKey}, {"ollama with key", llm.Settings{Provider: "ollama", BaseURL: "http://o:11434", Model: "m"}, testKey, llm.ErrUnexpectedKey}, {"anthropic key with newline", llm.Settings{Provider: "anthropic", Model: "m"}, testKey + "\n", llm.ErrInvalidKey}, {"openai key with newline", llm.Settings{Provider: "openai", Model: "m"}, testKey + "\n", llm.ErrInvalidKey}, {"openrouter key with CR LF", llm.Settings{Provider: "openrouter", Model: "m", PublicURL: "https://x.example"}, testKey + "\r\nX-Injected: 1", llm.ErrInvalidKey}, + {"xai key with newline", llm.Settings{Provider: "xai", Model: "grok-4"}, testKey + "\n", llm.ErrInvalidKey}, {"openai_compatible key with NUL", llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1", Model: "m"}, "\x00" + testKey, llm.ErrInvalidKey}, {"ollama no base URL", llm.Settings{Provider: "ollama", Model: "m"}, "", llm.ErrMissingBaseURL}, {"openai_compatible no base URL", llm.Settings{Provider: "openai_compatible", Model: "m"}, "", llm.ErrMissingBaseURL}, @@ -362,8 +402,11 @@ func TestConfigErrors(t *testing.T) { {"openrouter no model", llm.Settings{Provider: "openrouter", PublicURL: "https://x.example"}, testKey, llm.ErrMissingModel}, {"ollama no model", llm.Settings{Provider: "ollama", BaseURL: "http://o:11434"}, "", llm.ErrMissingModel}, {"openai_compatible no model", llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1"}, "", llm.ErrMissingModel}, + {"xai no model", llm.Settings{Provider: "xai"}, testKey, llm.ErrMissingModel}, {"openrouter no public URL", llm.Settings{Provider: "openrouter", Model: "m"}, testKey, llm.ErrMissingPublicURL}, {"fixed base URL overridden", llm.Settings{Provider: "openai", BaseURL: "https://evil.example/v1", Model: "m"}, testKey, llm.ErrInvalidSetting}, + {"xai base URL overridden", llm.Settings{Provider: "xai", BaseURL: "https://api.openai.com/v1", Model: "grok-4"}, testKey, llm.ErrInvalidSetting}, + {"xai prompt_only", llm.Settings{Provider: "xai", Model: "grok-4", StructuredOutput: "prompt_only"}, testKey, llm.ErrInvalidSetting}, {"openai json_object", llm.Settings{Provider: "openai", Model: "m", StructuredOutput: "json_object"}, testKey, llm.ErrInvalidSetting}, {"anthropic mode", llm.Settings{Provider: "anthropic", Model: "m", StructuredOutput: "json_schema"}, testKey, llm.ErrInvalidSetting}, {"unknown mode", llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1", Model: "m", StructuredOutput: "xml"}, "", llm.ErrInvalidSetting}, @@ -396,6 +439,7 @@ func TestReadyProviderForEveryPreset(t *testing.T) { {llm.Settings{Provider: "anthropic", Model: "m"}, testKey}, {llm.Settings{Provider: "openai", Model: "m"}, testKey}, {llm.Settings{Provider: "openrouter", Model: "m", PublicURL: "https://x.example"}, testKey}, + {llm.Settings{Provider: "xai", Model: "m"}, testKey}, {llm.Settings{Provider: "ollama", BaseURL: "http://o:11434", Model: "m"}, ""}, {llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1", Model: "m"}, ""}, } { @@ -454,6 +498,10 @@ func TestConfigErrorsNeverLeakKey(t *testing.T) { {"key in bad base URL", llm.Settings{Provider: "openai_compatible", BaseURL: "ftp://k3y", Model: "m"}, "k3y", llm.ErrInvalidSetting}, {"ollama with key", llm.Settings{Provider: "ollama", BaseURL: "http://o:11434", Model: "m"}, "key", llm.ErrUnexpectedKey}, {"key with newline", llm.Settings{Provider: "openai", Model: "m"}, "sk-test\n", llm.ErrInvalidKey}, + {"xai key as preset name", llm.Settings{Provider: "xai"}, "xai", llm.ErrMissingModel}, + {"xai key as missing-model text", llm.Settings{Provider: "xai"}, "model is required", llm.ErrMissingModel}, + {"xai key as fixed-URL text", llm.Settings{Provider: "xai", BaseURL: "https://x.example/v1", Model: "m"}, "fixed", llm.ErrInvalidSetting}, + {"xai key as structured-output text", llm.Settings{Provider: "xai", Model: "m", StructuredOutput: "prompt_only"}, "structured output", llm.ErrInvalidSetting}, {"key with newline as text", llm.Settings{Provider: "openai", Model: "m"}, "header\n", llm.ErrInvalidKey}, } for _, c := range cases { diff --git a/internal/ai/llm/settings.go b/internal/ai/llm/settings.go index adde36c..11c2c74 100644 --- a/internal/ai/llm/settings.go +++ b/internal/ai/llm/settings.go @@ -18,7 +18,7 @@ import "errors" // to presets.FromSettings separately. type Settings struct { // Provider names the preset: "anthropic", "openai", "openrouter", - // "ollama" or "openai_compatible". + // "xai", "ollama" or "openai_compatible". Provider string // BaseURL is the API root. It is required for ollama and // openai_compatible; the other presets fix their own, and a non-empty diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index 444399a..624f17e 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -175,8 +175,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai", got) } } @@ -196,8 +196,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") @@ -396,3 +396,93 @@ func TestMigrateAIProviderPresetsKeepsRowsAndReferences(t *testing.T) { } } } + +// TestMigrateAIProviderXAIKeepsRowsAndReferences upgrades a database at 0004 +// holding ai_settings and ai_runs rows with 0004's providers, with +// suggestion_batches and week_plans referencing the run, and checks 0005 +// carries every row over, keeps the references enforced, accepts xai and +// still rejects an unknown provider. +func TestMigrateAIProviderXAIKeepsRowsAndReferences(t *testing.T) { + ctx := context.Background() + s := openTestStore(t) + before := fstest.MapFS{} + for _, name := range []string{"0000_bootstrap.sql", "0001_init.sql", "0002_household_settings_updated_by.sql", "0003_jobs_status.sql", "0004_ai_provider_presets.sql"} { + body, err := fs.ReadFile(migrations.FS, name) + if err != nil { + t.Fatal(err) + } + before[name] = &fstest.MapFile{Data: body} + } + if err := Migrate(ctx, s.Writer, before); err != nil { + t.Fatal(err) + } + for _, stmt := range []string{ + `INSERT INTO ai_settings (id, provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at, created_at, updated_at) + VALUES (1, 'openrouter', 'https://openrouter.ai/api/v1', 'openai/gpt-4o', 0.7, 'sp', x'0102', 'abcd', 'v', 'c', 'u')`, + `INSERT INTO ai_runs (id, skill, skill_version, provider, model, temperature, status, latency_ms, tokens_in, tokens_out, error, request_redacted, response_redacted, created_at, updated_at) + VALUES ('run-1', 'suggest', '1', 'openai', 'gpt-4o', 0.5, 'ok', 10, 20, 30, 'e', 'req', 'resp', 'c', 'u')`, + `INSERT INTO suggestion_batches (id, trigger, status, ai_run_id, created_at, updated_at) + VALUES ('batch-1', 'manual', 'done', 'run-1', 'c', 'u')`, + `INSERT INTO week_plans (id, week_start, status, ai_run_id, created_at, updated_at) + VALUES ('plan-1', '2026-10-04', 'draft', 'run-1', 'c', 'u')`, + } { + if _, err := s.Writer.Exec(stmt); err != nil { + t.Fatalf("seed: %v", err) + } + } + + if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + + var provider, baseURL, model, systemPrompt, last4, verifiedAt, createdAt, updatedAt string + var temperature float64 + var keyCT []byte + if err := s.Reader.QueryRow(`SELECT provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at, created_at, updated_at FROM ai_settings WHERE id = 1`). + Scan(&provider, &baseURL, &model, &temperature, &systemPrompt, &keyCT, &last4, &verifiedAt, &createdAt, &updatedAt); err != nil { + t.Fatalf("read ai_settings: %v", err) + } + settings := []any{provider, baseURL, model, temperature, systemPrompt, string(keyCT), last4, verifiedAt, createdAt, updatedAt} + if want := []any{"openrouter", "https://openrouter.ai/api/v1", "openai/gpt-4o", 0.7, "sp", "\x01\x02", "abcd", "v", "c", "u"}; !slices.Equal(settings, want) { + t.Errorf("ai_settings = %v, want %v", settings, want) + } + + var skill, skillVersion, status, errText, request, response string + var latency, tokensIn, tokensOut int + if err := s.Reader.QueryRow(`SELECT skill, skill_version, provider, model, temperature, status, latency_ms, tokens_in, tokens_out, error, request_redacted, response_redacted, created_at, updated_at FROM ai_runs WHERE id = 'run-1'`). + Scan(&skill, &skillVersion, &provider, &model, &temperature, &status, &latency, &tokensIn, &tokensOut, &errText, &request, &response, &createdAt, &updatedAt); err != nil { + t.Fatalf("read ai_runs: %v", err) + } + run := []any{skill, skillVersion, provider, model, temperature, status, latency, tokensIn, tokensOut, errText, request, response, createdAt, updatedAt} + if want := []any{"suggest", "1", "openai", "gpt-4o", 0.5, "ok", 10, 20, 30, "e", "req", "resp", "c", "u"}; !slices.Equal(run, want) { + t.Errorf("ai_runs = %v, want %v", run, want) + } + + rows, err := s.Reader.Query("PRAGMA foreign_key_check") + if err != nil { + t.Fatal(err) + } + if rows.Next() { + t.Error("foreign_key_check reported violations after 0005") + } + if err := rows.Close(); err != nil { + t.Fatal(err) + } + if _, err := s.Writer.Exec(`DELETE FROM ai_runs WHERE id = 'run-1'`); err == nil || !strings.Contains(err.Error(), "FOREIGN KEY constraint failed") { + t.Errorf("deleting a referenced ai_run: error %v, want FOREIGN KEY constraint failure", err) + } + + for _, table := range []string{"ai_settings", "ai_runs"} { + if _, err := s.Writer.Exec(`UPDATE ` + table + ` SET provider = 'xai'`); err != nil { + t.Errorf("%s.provider xai rejected: %v", table, err) + } + if _, err := s.Writer.Exec(`UPDATE ` + table + ` SET provider = 'grok'`); err == nil || !strings.Contains(err.Error(), "CHECK constraint failed") { + t.Errorf("%s.provider grok: error %v, want CHECK constraint failure", table, err) + } + } + for _, table := range []string{"ai_settings_copy", "ai_runs_copy"} { + if tableExists(t, s, table) { + t.Errorf("%s left behind", table) + } + } +} diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index 1ce39a4..a5651bf 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -364,7 +364,7 @@ func TestSchemaRotationReviewEventAllowed(t *testing.T) { // both ai_settings and ai_runs. func TestSchemaAIProvidersAllowed(t *testing.T) { s := seededStore(t) - for _, provider := range []string{"openai_compatible", "anthropic", "ollama", "openai", "openrouter"} { + for _, provider := range []string{"openai_compatible", "anthropic", "ollama", "openai", "openrouter", "xai"} { for _, table := range []string{"ai_settings", "ai_runs"} { inRolledBackTx(t, s, func(tx *sql.Tx) { row := testRow(table, 1) diff --git a/migrations/0005_ai_provider_xai.sql b/migrations/0005_ai_provider_xai.sql new file mode 100644 index 0000000..6976934 --- /dev/null +++ b/migrations/0005_ai_provider_xai.sql @@ -0,0 +1,64 @@ +-- 0005_ai_provider_xai: ai_settings.provider and ai_runs.provider also +-- accept xai, the xAI Grok preset in internal/ai/llm/presets. Every 0004 +-- value is kept, so existing rows copy unchanged. The rebuild is 0004's: +-- copy the rows aside, drop the table, create it again with the wider CHECK, +-- copy the rows back and drop the copy. Neither table has indexes or +-- triggers. +-- +-- suggestion_batches.ai_run_id and week_plans.ai_run_id reference ai_runs, +-- so foreign keys are deferred until commit, after the ai_runs rows are +-- back. The pragma lasts only for this migration's transaction. + +PRAGMA defer_foreign_keys = ON; + +CREATE TABLE ai_settings_copy AS SELECT * FROM ai_settings; + +DROP TABLE ai_settings; + +CREATE TABLE ai_settings ( + id INTEGER PRIMARY KEY CHECK (id = 1), + provider TEXT NOT NULL CHECK (provider IN ('openai_compatible', 'anthropic', 'ollama', 'openai', 'openrouter', 'xai')), + base_url TEXT NOT NULL, + model TEXT NOT NULL, + temperature REAL NOT NULL, + system_prompt TEXT, + api_key_ct BLOB, + api_key_last4 TEXT, + key_verified_at TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL +); + +INSERT INTO ai_settings (id, provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at, created_at, updated_at) +SELECT id, provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at, created_at, updated_at +FROM ai_settings_copy; + +DROP TABLE ai_settings_copy; + +CREATE TABLE ai_runs_copy AS SELECT * FROM ai_runs; + +DROP TABLE ai_runs; + +CREATE TABLE ai_runs ( + id TEXT NOT NULL PRIMARY KEY, + skill TEXT NOT NULL, + skill_version TEXT NOT NULL, + provider TEXT NOT NULL CHECK (provider IN ('openai_compatible', 'anthropic', 'ollama', 'openai', 'openrouter', 'xai')), + model TEXT NOT NULL, + temperature REAL NOT NULL, + status TEXT NOT NULL CHECK (status IN ('running', 'ok', 'error')), + latency_ms INTEGER, + tokens_in INTEGER, + tokens_out INTEGER, + error TEXT, + request_redacted TEXT NOT NULL, + response_redacted TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL +); + +INSERT INTO ai_runs (id, skill, skill_version, provider, model, temperature, status, latency_ms, tokens_in, tokens_out, error, request_redacted, response_redacted, created_at, updated_at) +SELECT id, skill, skill_version, provider, model, temperature, status, latency_ms, tokens_in, tokens_out, error, request_redacted, response_redacted, created_at, updated_at +FROM ai_runs_copy; + +DROP TABLE ai_runs_copy; -- 2.55.0