diff --git a/cmd/config/droid.go b/cmd/config/droid.go index ece40d682..75e48d02f 100644 --- a/cmd/config/droid.go +++ b/cmd/config/droid.go @@ -7,14 +7,23 @@ import ( "os/exec" "path/filepath" "slices" - "strings" ) // Droid implements Runner and Editor for Droid integration type Droid struct{} -// droidModelEntry represents a custom model entry in Droid's settings.json -type droidModelEntry struct { +// droidSettings represents the Droid settings.json file (only fields we use) +type droidSettings struct { + CustomModels []modelEntry `json:"customModels"` + SessionDefaultSettings sessionSettings `json:"sessionDefaultSettings"` +} + +type sessionSettings struct { + Model string `json:"model"` + ReasoningEffort string `json:"reasoningEffort"` +} + +type modelEntry struct { Model string `json:"model"` DisplayName string `json:"displayName"` BaseURL string `json:"baseUrl"` @@ -76,24 +85,36 @@ func (d *Droid) Edit(models []string) error { return err } - settings := make(map[string]any) + // Read file once, unmarshal twice: + // map preserves unknown fields for writing back (including extra fields in model entries) + settingsMap := make(map[string]any) + var settings droidSettings if data, err := os.ReadFile(settingsPath); err == nil { - if err := json.Unmarshal(data, &settings); err != nil { + if err := json.Unmarshal(data, &settingsMap); err != nil { return fmt.Errorf("failed to parse settings file: %w, at: %s", err, settingsPath) } + json.Unmarshal(data, &settings) // ignore error, zero values are fine + } + + // Keep only non-Ollama models from the raw map (preserves extra fields) + // Rebuild Ollama models + var nonOllamaModels []any + if rawModels, ok := settingsMap["customModels"].([]any); ok { + for _, raw := range rawModels { + if m, ok := raw.(map[string]any); ok { + if m["apiKey"] != "ollama" { + nonOllamaModels = append(nonOllamaModels, raw) + } + } + } } - customModels, _ := settings["customModels"].([]any) - - // Keep only non-Ollama models (we'll rebuild Ollama models fresh) - nonOllamaModels := slices.DeleteFunc(slices.Clone(customModels), isOllamaModelEntry) - // Build new Ollama model entries with sequential indices (0, 1, 2, ...) - var ollamaModels []any + var newModels []any var defaultModelID string for i, model := range models { - modelID := fmt.Sprintf("custom:%s-[Ollama]-%d", model, i) - ollamaModels = append(ollamaModels, droidModelEntry{ + modelID := fmt.Sprintf("custom:%s-%d", model, i) + newModels = append(newModels, modelEntry{ Model: model, DisplayName: model, BaseURL: "http://localhost:11434/v1", @@ -109,21 +130,22 @@ func (d *Droid) Edit(models []string) error { } } - settings["customModels"] = append(ollamaModels, nonOllamaModels...) + settingsMap["customModels"] = append(newModels, nonOllamaModels...) - sessionSettings, ok := settings["sessionDefaultSettings"].(map[string]any) + // Update session default settings (preserve unknown fields in the nested object) + sessionSettings, ok := settingsMap["sessionDefaultSettings"].(map[string]any) if !ok { sessionSettings = make(map[string]any) } sessionSettings["model"] = defaultModelID - if effort, ok := sessionSettings["reasoningEffort"].(string); !ok || !isValidReasoningEffort(effort) { + if !isValidReasoningEffort(settings.SessionDefaultSettings.ReasoningEffort) { sessionSettings["reasoningEffort"] = "none" } - settings["sessionDefaultSettings"] = sessionSettings + settingsMap["sessionDefaultSettings"] = sessionSettings - data, err := json.MarshalIndent(settings, "", " ") + data, err := json.MarshalIndent(settingsMap, "", " ") if err != nil { return err } @@ -135,24 +157,21 @@ func (d *Droid) Models() []string { if err != nil { return nil } - settings, err := readJSONFile(filepath.Join(home, ".factory", "settings.json")) + + data, err := os.ReadFile(filepath.Join(home, ".factory", "settings.json")) if err != nil { return nil } - customModels, _ := settings["customModels"].([]any) + var settings droidSettings + if err := json.Unmarshal(data, &settings); err != nil { + return nil + } var result []string - for _, m := range customModels { - if !isOllamaModelEntry(m) { - continue - } - entry, ok := m.(map[string]any) - if !ok { - continue - } - if model, _ := entry["model"].(string); model != "" { - result = append(result, model) + for _, m := range settings.CustomModels { + if m.APIKey == "ollama" { + result = append(result, m.Model) } } return result @@ -163,12 +182,3 @@ var validReasoningEfforts = []string{"high", "medium", "low", "none"} func isValidReasoningEffort(effort string) bool { return slices.Contains(validReasoningEfforts, effort) } - -func isOllamaModelEntry(m any) bool { - entry, ok := m.(map[string]any) - if !ok { - return false - } - id, _ := entry["id"].(string) - return strings.Contains(id, "-[Ollama]-") -} diff --git a/cmd/config/droid_test.go b/cmd/config/droid_test.go index d158ad3a5..9d202fc98 100644 --- a/cmd/config/droid_test.go +++ b/cmd/config/droid_test.go @@ -2,6 +2,7 @@ package config import ( "encoding/json" + "fmt" "os" "path/filepath" "testing" @@ -75,8 +76,8 @@ func TestDroidEdit(t *testing.T) { if models[0]["model"] != "model-a" { t.Errorf("expected model-a, got %s", models[0]["model"]) } - if models[0]["id"] != "custom:model-a-[Ollama]-0" { - t.Errorf("expected custom:model-a-[Ollama]-0, got %s", models[0]["id"]) + if models[0]["id"] != "custom:model-a-0" { + t.Errorf("expected custom:model-a-0, got %s", models[0]["id"]) } if models[0]["index"] != float64(0) { t.Errorf("expected index 0, got %v", models[0]["index"]) @@ -86,8 +87,8 @@ func TestDroidEdit(t *testing.T) { if models[1]["model"] != "model-b" { t.Errorf("expected model-b, got %s", models[1]["model"]) } - if models[1]["id"] != "custom:model-b-[Ollama]-1" { - t.Errorf("expected custom:model-b-[Ollama]-1, got %s", models[1]["id"]) + if models[1]["id"] != "custom:model-b-1" { + t.Errorf("expected custom:model-b-1, got %s", models[1]["id"]) } if models[1]["index"] != float64(1) { t.Errorf("expected index 1, got %v", models[1]["index"]) @@ -105,8 +106,8 @@ func TestDroidEdit(t *testing.T) { if !ok { t.Fatal("sessionDefaultSettings not found") } - if session["model"] != "custom:model-a-[Ollama]-0" { - t.Errorf("expected custom:model-a-[Ollama]-0, got %s", session["model"]) + if session["model"] != "custom:model-a-0" { + t.Errorf("expected custom:model-a-0, got %s", session["model"]) } }) @@ -134,11 +135,11 @@ func TestDroidEdit(t *testing.T) { } // Check IDs match new indices - if models[0]["id"] != "custom:model-a-[Ollama]-0" { - t.Errorf("expected custom:model-a-[Ollama]-0, got %s", models[0]["id"]) + if models[0]["id"] != "custom:model-a-0" { + t.Errorf("expected custom:model-a-0, got %s", models[0]["id"]) } - if models[1]["id"] != "custom:model-c-[Ollama]-1" { - t.Errorf("expected custom:model-c-[Ollama]-1, got %s", models[1]["id"]) + if models[1]["id"] != "custom:model-c-1" { + t.Errorf("expected custom:model-c-1, got %s", models[1]["id"]) } }) @@ -390,13 +391,13 @@ func TestDroidEdit_MalformedModelEntry(t *testing.T) { t.Fatalf("Edit with malformed entries failed: %v", err) } - // Malformed entries should be preserved in nonOllamaModels + // Malformed entries (non-object) are dropped - only valid model objects are preserved settings, _ := readJSONFile(settingsPath) customModels, _ := settings["customModels"].([]any) - // Should have: 1 new Ollama model + 2 preserved malformed entries - if len(customModels) != 3 { - t.Errorf("expected 3 entries (1 new + 2 preserved malformed), got %d", len(customModels)) + // Should have: 1 new Ollama model only (malformed entries dropped) + if len(customModels) != 1 { + t.Errorf("expected 1 entry (malformed entries dropped), got %d", len(customModels)) } } @@ -428,6 +429,253 @@ func TestDroidEdit_WrongTypeSessionSettings(t *testing.T) { } } +// testDroidSettingsFixture is a representative settings.json fixture for testing. +// It covers: simple fields, arrays, nested objects, and customModels. +const testDroidSettingsFixture = `{ + "commandAllowlist": ["ls", "pwd", "git status"], + "diffMode": "github", + "enableHooks": true, + "hooks": { + "claudeHooksImported": true, + "importedClaudeHooks": ["uv run ruff check", "echo test"] + }, + "ideExtensionPromptedAt": { + "cursor": 1763081579486, + "vscode": 1762992990179 + }, + "customModels": [ + { + "model": "existing-ollama-model", + "displayName": "existing-ollama-model", + "baseUrl": "http://localhost:11434/v1", + "apiKey": "ollama", + "provider": "generic-chat-completion-api", + "maxOutputTokens": 64000, + "supportsImages": false, + "id": "custom:existing-ollama-model-0", + "index": 0 + }, + { + "model": "gpt-4", + "displayName": "GPT-4", + "baseUrl": "https://api.openai.com/v1", + "apiKey": "sk-xxx", + "provider": "openai", + "maxOutputTokens": 4096, + "supportsImages": true, + "id": "openai-gpt4", + "index": 1, + "customField": "should be preserved" + } + ], + "sessionDefaultSettings": { + "autonomyMode": "auto-medium", + "model": "custom:existing-ollama-model-0", + "reasoningEffort": "high" + }, + "todoDisplayMode": "pinned" +}` + +func TestDroidEdit_RoundTrip(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + os.WriteFile(settingsPath, []byte(testDroidSettingsFixture), 0o644) + + // Edit with new models + if err := d.Edit([]string{"llama3", "mistral"}); err != nil { + t.Fatal(err) + } + + // Read back and verify + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + // Verify unknown top-level fields preserved + if settings["diffMode"] != "github" { + t.Error("diffMode not preserved") + } + if settings["enableHooks"] != true { + t.Error("enableHooks not preserved") + } + if settings["todoDisplayMode"] != "pinned" { + t.Error("todoDisplayMode not preserved") + } + + // Verify arrays preserved + allowlist, ok := settings["commandAllowlist"].([]any) + if !ok || len(allowlist) != 3 { + t.Error("commandAllowlist not preserved") + } + + // Verify nested objects preserved + hooks, ok := settings["hooks"].(map[string]any) + if !ok { + t.Fatal("hooks not preserved") + } + if hooks["claudeHooksImported"] != true { + t.Error("hooks.claudeHooksImported not preserved") + } + importedHooks, ok := hooks["importedClaudeHooks"].([]any) + if !ok || len(importedHooks) != 2 { + t.Error("hooks.importedClaudeHooks not preserved") + } + + // Verify deeply nested numeric values preserved + idePrompted, ok := settings["ideExtensionPromptedAt"].(map[string]any) + if !ok { + t.Fatal("ideExtensionPromptedAt not preserved") + } + if idePrompted["cursor"] != float64(1763081579486) { + t.Error("ideExtensionPromptedAt.cursor not preserved") + } + + // Verify sessionDefaultSettings unknown fields preserved + session, ok := settings["sessionDefaultSettings"].(map[string]any) + if !ok { + t.Fatal("sessionDefaultSettings not preserved") + } + if session["autonomyMode"] != "auto-medium" { + t.Error("sessionDefaultSettings.autonomyMode not preserved") + } + if session["reasoningEffort"] != "high" { + t.Error("sessionDefaultSettings.reasoningEffort not preserved (was valid)") + } + // model should be updated + if session["model"] != "custom:llama3-0" { + t.Errorf("sessionDefaultSettings.model not updated, got %s", session["model"]) + } + + // Verify customModels: old ollama replaced, non-ollama preserved with extra fields + models, ok := settings["customModels"].([]any) + if !ok { + t.Fatal("customModels not preserved") + } + if len(models) != 3 { // 2 new ollama + 1 non-ollama + t.Fatalf("expected 3 models, got %d", len(models)) + } + + // First two should be new Ollama models + m0 := models[0].(map[string]any) + if m0["model"] != "llama3" || m0["apiKey"] != "ollama" { + t.Error("first model should be llama3") + } + m1 := models[1].(map[string]any) + if m1["model"] != "mistral" || m1["apiKey"] != "ollama" { + t.Error("second model should be mistral") + } + + // Third should be preserved non-Ollama with extra field + m2 := models[2].(map[string]any) + if m2["model"] != "gpt-4" { + t.Error("non-Ollama model not preserved") + } + if m2["customField"] != "should be preserved" { + t.Error("non-Ollama model's extra field not preserved") + } +} + +func TestDroidEdit_PreservesUnknownFields(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + readSettings := func() map[string]any { + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + return settings + } + + t.Run("preserves all JSON value types", func(t *testing.T) { + os.RemoveAll(settingsDir) + os.MkdirAll(settingsDir, 0o755) + + original := `{ + "stringField": "value", + "numberField": 42, + "floatField": 3.14, + "boolField": true, + "nullField": null, + "arrayField": [1, "two", true], + "objectField": {"nested": "value"}, + "customModels": [], + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + settings := readSettings() + + if settings["stringField"] != "value" { + t.Error("stringField not preserved") + } + if settings["numberField"] != float64(42) { + t.Error("numberField not preserved") + } + if settings["floatField"] != 3.14 { + t.Error("floatField not preserved") + } + if settings["boolField"] != true { + t.Error("boolField not preserved") + } + if settings["nullField"] != nil { + t.Error("nullField not preserved") + } + arr, ok := settings["arrayField"].([]any) + if !ok || len(arr) != 3 { + t.Error("arrayField not preserved") + } + obj, ok := settings["objectField"].(map[string]any) + if !ok || obj["nested"] != "value" { + t.Error("objectField not preserved") + } + }) + + t.Run("preserves extra fields in non-Ollama models", func(t *testing.T) { + os.RemoveAll(settingsDir) + os.MkdirAll(settingsDir, 0o755) + + original := `{ + "customModels": [{ + "model": "gpt-4", + "apiKey": "sk-xxx", + "extraField": "preserved", + "nestedExtra": {"foo": "bar"} + }] + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"llama3"}); err != nil { + t.Fatal(err) + } + + settings := readSettings() + models := settings["customModels"].([]any) + gpt4 := models[1].(map[string]any) // non-Ollama is second + + if gpt4["extraField"] != "preserved" { + t.Error("extraField not preserved") + } + nested := gpt4["nestedExtra"].(map[string]any) + if nested["foo"] != "bar" { + t.Error("nestedExtra not preserved") + } + }) +} + func TestIsValidReasoningEffort(t *testing.T) { tests := []struct { effort string @@ -452,3 +700,603 @@ func TestIsValidReasoningEffort(t *testing.T) { }) } } + +func TestDroidEdit_Idempotent(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + os.WriteFile(settingsPath, []byte(testDroidSettingsFixture), 0o644) + + // Edit twice with same models + d.Edit([]string{"llama3", "mistral"}) + firstData, _ := os.ReadFile(settingsPath) + + d.Edit([]string{"llama3", "mistral"}) + secondData, _ := os.ReadFile(settingsPath) + + // Results should be identical + if string(firstData) != string(secondData) { + t.Error("repeated edits with same models produced different results") + } +} + +func TestDroidEdit_MultipleConsecutiveEdits(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + os.WriteFile(settingsPath, []byte(testDroidSettingsFixture), 0o644) + + // Multiple edits shouldn't accumulate garbage or lose data + for i := range 10 { + models := []string{"model-a", "model-b"} + if i%2 == 0 { + models = []string{"model-x", "model-y", "model-z"} + } + if err := d.Edit(models); err != nil { + t.Fatalf("edit %d failed: %v", i, err) + } + } + + // Verify file is still valid JSON and preserves original fields + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + if err := json.Unmarshal(data, &settings); err != nil { + t.Fatalf("file is not valid JSON after multiple edits: %v", err) + } + + // Original fields should still be there + if settings["diffMode"] != "github" { + t.Error("diffMode lost after multiple edits") + } + if settings["enableHooks"] != true { + t.Error("enableHooks lost after multiple edits") + } + + // Non-Ollama model should still be preserved + models := settings["customModels"].([]any) + foundOther := false + for _, m := range models { + if entry, ok := m.(map[string]any); ok { + if entry["model"] == "gpt-4" { + foundOther = true + if entry["customField"] != "should be preserved" { + t.Error("other customField lost after multiple edits") + } + } + } + } + if !foundOther { + t.Error("other model lost after multiple edits") + } +} + +func TestDroidEdit_UnicodeAndSpecialCharacters(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + // Settings with unicode and special characters + original := `{ + "userName": "日本語テスト", + "emoji": "🚀🎉💻", + "specialChars": "quotes: \"test\" and 'test', backslash: \\, newline: \n, tab: \t", + "unicodeEscape": "\u0048\u0065\u006c\u006c\u006f", + "customModels": [], + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + if settings["userName"] != "日本語テスト" { + t.Error("Japanese characters not preserved") + } + if settings["emoji"] != "🚀🎉💻" { + t.Error("emoji not preserved") + } + // Note: JSON encoding will normalize escape sequences + if settings["unicodeEscape"] != "Hello" { + t.Error("unicode escape sequence not preserved") + } +} + +func TestDroidEdit_LargeNumbers(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + // Large numbers and timestamps (common in settings files) + original := `{ + "timestamp": 1763081579486, + "largeInt": 9007199254740991, + "negativeNum": -12345, + "floatNum": 3.141592653589793, + "scientificNotation": 1.23e10, + "customModels": [], + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + if settings["timestamp"] != float64(1763081579486) { + t.Errorf("timestamp not preserved: got %v", settings["timestamp"]) + } + if settings["largeInt"] != float64(9007199254740991) { + t.Errorf("largeInt not preserved: got %v", settings["largeInt"]) + } + if settings["negativeNum"] != float64(-12345) { + t.Error("negativeNum not preserved") + } + if settings["floatNum"] != 3.141592653589793 { + t.Error("floatNum not preserved") + } +} + +func TestDroidEdit_EmptyAndNullValues(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + original := `{ + "emptyString": "", + "nullValue": null, + "emptyArray": [], + "emptyObject": {}, + "falseBool": false, + "zeroNumber": 0, + "customModels": [], + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + if settings["emptyString"] != "" { + t.Error("emptyString not preserved") + } + if settings["nullValue"] != nil { + t.Error("nullValue not preserved as null") + } + if arr, ok := settings["emptyArray"].([]any); !ok || len(arr) != 0 { + t.Error("emptyArray not preserved") + } + if obj, ok := settings["emptyObject"].(map[string]any); !ok || len(obj) != 0 { + t.Error("emptyObject not preserved") + } + if settings["falseBool"] != false { + t.Error("falseBool not preserved (false vs missing)") + } + if settings["zeroNumber"] != float64(0) { + t.Error("zeroNumber not preserved") + } +} + +func TestDroidEdit_DeeplyNestedStructures(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + original := `{ + "level1": { + "level2": { + "level3": { + "level4": { + "deepValue": "found me", + "deepArray": [1, 2, {"nested": true}] + } + } + } + }, + "customModels": [], + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + // Navigate to deeply nested value + l1 := settings["level1"].(map[string]any) + l2 := l1["level2"].(map[string]any) + l3 := l2["level3"].(map[string]any) + l4 := l3["level4"].(map[string]any) + + if l4["deepValue"] != "found me" { + t.Error("deeply nested value not preserved") + } + + deepArray := l4["deepArray"].([]any) + if len(deepArray) != 3 { + t.Error("deeply nested array not preserved") + } + nestedInArray := deepArray[2].(map[string]any) + if nestedInArray["nested"] != true { + t.Error("object nested in array not preserved") + } +} + +func TestDroidEdit_ModelNamesWithSpecialCharacters(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + // Test model names with colons, slashes, special chars + specialModels := []string{ + "qwen3:480b-cloud", + "llama3.2:70b", + "model/with/slashes", + "model-with-dashes", + "model_with_underscores", + } + + if err := d.Edit(specialModels); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + models := settings["customModels"].([]any) + if len(models) != len(specialModels) { + t.Fatalf("expected %d models, got %d", len(specialModels), len(models)) + } + + for i, expected := range specialModels { + m := models[i].(map[string]any) + if m["model"] != expected { + t.Errorf("model %d: expected %s, got %s", i, expected, m["model"]) + } + } +} + +func TestDroidEdit_MissingCustomModelsKey(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + // No customModels key at all + original := `{ + "diffMode": "github", + "sessionDefaultSettings": {"autonomyMode": "auto-high"} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + // Original fields preserved + if settings["diffMode"] != "github" { + t.Error("diffMode not preserved") + } + + // customModels created + models, ok := settings["customModels"].([]any) + if !ok || len(models) != 1 { + t.Error("customModels not created properly") + } +} + +func TestDroidEdit_NullCustomModels(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + original := `{ + "customModels": null, + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + models, ok := settings["customModels"].([]any) + if !ok || len(models) != 1 { + t.Error("null customModels not handled properly") + } +} + +func TestDroidEdit_MinifiedJSON(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + // Minified JSON (no whitespace) + original := `{"diffMode":"github","enableHooks":true,"hooks":{"imported":["cmd1","cmd2"]},"customModels":[],"sessionDefaultSettings":{}}` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + if err := json.Unmarshal(data, &settings); err != nil { + t.Fatal("output is not valid JSON") + } + + if settings["diffMode"] != "github" { + t.Error("diffMode not preserved from minified JSON") + } + if settings["enableHooks"] != true { + t.Error("enableHooks not preserved from minified JSON") + } +} + +func TestDroidEdit_CreatesDirectoryIfMissing(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + + // Directory doesn't exist + if _, err := os.Stat(settingsDir); !os.IsNotExist(err) { + t.Fatal("directory should not exist before test") + } + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + // Directory should be created + if _, err := os.Stat(settingsDir); os.IsNotExist(err) { + t.Fatal("directory was not created") + } + + // File should exist and be valid + settingsPath := filepath.Join(settingsDir, "settings.json") + data, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatal("settings file not created") + } + + var settings map[string]any + if err := json.Unmarshal(data, &settings); err != nil { + t.Fatal("created file is not valid JSON") + } +} + +func TestDroidEdit_PreservesFileAfterError(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + // Valid original content + original := `{"diffMode": "github", "customModels": [], "sessionDefaultSettings": {}}` + os.WriteFile(settingsPath, []byte(original), 0o644) + + // Empty models list is a no-op, should not modify file + d.Edit([]string{}) + + data, _ := os.ReadFile(settingsPath) + if string(data) != original { + t.Error("file was modified when it should not have been") + } +} + +func TestDroidEdit_BackupCreated(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + backupDir := filepath.Join(os.TempDir(), "ollama-backups") + + os.MkdirAll(settingsDir, 0o755) + + // Use a unique marker to identify our backup + uniqueMarker := fmt.Sprintf("test-marker-%d", os.Getpid()) + original := fmt.Sprintf(`{"diffMode": "%s", "customModels": [], "sessionDefaultSettings": {}}`, uniqueMarker) + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + // Find backup containing our unique marker + backups, _ := filepath.Glob(filepath.Join(backupDir, "settings.json.*")) + foundBackup := false + for _, backup := range backups { + data, err := os.ReadFile(backup) + if err != nil { + continue + } + if string(data) == original { + foundBackup = true + break + } + } + + if !foundBackup { + t.Error("backup with original content not found") + } + + // Main file should be modified + newData, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(newData, &settings) + + models := settings["customModels"].([]any) + if len(models) != 1 { + t.Error("main file was not updated") + } +} + +func TestDroidEdit_LargeNumberOfModels(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + os.WriteFile(settingsPath, []byte(`{"customModels": [], "sessionDefaultSettings": {}}`), 0o644) + + // Add many models + var models []string + for i := range 100 { + models = append(models, fmt.Sprintf("model-%d", i)) + } + + if err := d.Edit(models); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + customModels := settings["customModels"].([]any) + if len(customModels) != 100 { + t.Errorf("expected 100 models, got %d", len(customModels)) + } + + // Verify indices are correct + for i, m := range customModels { + entry := m.(map[string]any) + if entry["index"] != float64(i) { + t.Errorf("model %d has wrong index: %v", i, entry["index"]) + } + } +} + +func TestDroidEdit_ArraysWithMixedTypes(t *testing.T) { + d := &Droid{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + settingsDir := filepath.Join(tmpDir, ".factory") + settingsPath := filepath.Join(settingsDir, "settings.json") + + os.MkdirAll(settingsDir, 0o755) + + // Arrays with mixed types (valid JSON) + original := `{ + "mixedArray": [1, "two", true, null, {"nested": "obj"}, [1,2,3]], + "customModels": [], + "sessionDefaultSettings": {} + }` + os.WriteFile(settingsPath, []byte(original), 0o644) + + if err := d.Edit([]string{"model-a"}); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(settingsPath) + var settings map[string]any + json.Unmarshal(data, &settings) + + arr := settings["mixedArray"].([]any) + if len(arr) != 6 { + t.Error("mixedArray length not preserved") + } + if arr[0] != float64(1) { + t.Error("number in mixed array not preserved") + } + if arr[1] != "two" { + t.Error("string in mixed array not preserved") + } + if arr[2] != true { + t.Error("bool in mixed array not preserved") + } + if arr[3] != nil { + t.Error("null in mixed array not preserved") + } + if nested, ok := arr[4].(map[string]any); !ok || nested["nested"] != "obj" { + t.Error("object in mixed array not preserved") + } + if innerArr, ok := arr[5].([]any); !ok || len(innerArr) != 3 { + t.Error("array in mixed array not preserved") + } +} diff --git a/cmd/config/integrations.go b/cmd/config/integrations.go index e897317d0..bce1cfa45 100644 --- a/cmd/config/integrations.go +++ b/cmd/config/integrations.go @@ -34,6 +34,7 @@ type Editor interface { // Edit updates the config files for the integration with the given models Edit(models []string) error // Models returns the models currently configured for the integration + // TODO(parthsareen): add error return to Models() Models() []string }