cmd: ollama config fix droid model name configuration (#13856)

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]-")
-}

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")
+	}
+}

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
 }