diff --git a/cmd/launch/integrations_test.go b/cmd/launch/integrations_test.go index 8be31592a..5234f89fd 100644 --- a/cmd/launch/integrations_test.go +++ b/cmd/launch/integrations_test.go @@ -8,18 +8,23 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" + "path/filepath" "slices" "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/ollama/ollama/api" + "github.com/ollama/ollama/cmd/internal/fileutil" ) type stubEditorRunner struct { edited [][]string ranModel string editErr error + paths []string + editFn func(models []string) error } func (s *stubEditorRunner) Run(model string, args []string) error { @@ -29,12 +34,17 @@ func (s *stubEditorRunner) Run(model string, args []string) error { func (s *stubEditorRunner) String() string { return "StubEditor" } -func (s *stubEditorRunner) Paths() []string { return nil } +func (s *stubEditorRunner) Paths() []string { return append([]string(nil), s.paths...) } func (s *stubEditorRunner) Edit(models []string) error { if s.editErr != nil { return s.editErr } + if s.editFn != nil { + if err := s.editFn(models); err != nil { + return err + } + } cloned := append([]string(nil), models...) s.edited = append(s.edited, cloned) return nil @@ -745,6 +755,62 @@ func TestPrepareEditorIntegration_SavesOnlyAfterSuccessfulEdit(t *testing.T) { } } +func TestPrepareEditorIntegration_ShowsBackupWarningWhenConfigChanges(t *testing.T) { + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + configPath := filepath.Join(tmpDir, "settings.json") + original := []byte(`{"model":"old"}`) + if err := os.WriteFile(configPath, original, 0o644); err != nil { + t.Fatalf("failed to seed config file: %v", err) + } + + editor := &stubEditorRunner{ + paths: []string{configPath}, + editFn: func(models []string) error { + return fileutil.WriteWithBackup(configPath, []byte(`{"model":"new"}`)) + }, + } + + stderr := captureStderr(t, func() { + if err := prepareEditorIntegration("opencode", editor, editor, []string{"llama3.2"}); err != nil { + t.Fatalf("prepareEditorIntegration returned error: %v", err) + } + }) + + if !strings.Contains(stderr, "configuration has been modified. Backups are saved in") { + t.Fatalf("expected backup warning, got stderr: %q", stderr) + } +} + +func TestPrepareEditorIntegration_DoesNotShowBackupWarningWhenConfigUnchanged(t *testing.T) { + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + configPath := filepath.Join(tmpDir, "settings.json") + original := []byte(`{"model":"same"}`) + if err := os.WriteFile(configPath, original, 0o644); err != nil { + t.Fatalf("failed to seed config file: %v", err) + } + + editor := &stubEditorRunner{ + paths: []string{configPath}, + editFn: func(models []string) error { + return fileutil.WriteWithBackup(configPath, original) + }, + } + + stderr := captureStderr(t, func() { + if err := prepareEditorIntegration("pi", editor, editor, []string{"llama3.2"}); err != nil { + t.Fatalf("prepareEditorIntegration returned error: %v", err) + } + }) + + if strings.Contains(stderr, "configuration has been modified. Backups are saved in") { + t.Fatalf("did not expect backup warning when config is unchanged, got stderr: %q", stderr) + } +} + func TestShowOrPull_ModelExists(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/show" { diff --git a/cmd/launch/launch.go b/cmd/launch/launch.go index dd65466f8..96323476a 100644 --- a/cmd/launch/launch.go +++ b/cmd/launch/launch.go @@ -504,6 +504,8 @@ func (c *launcherClient) launchEditorIntegration(ctx context.Context, name strin if err := prepareEditorIntegration(name, runner, editor, models); err != nil { return err } + + return launchAfterConfiguration(name, runner, models[0], req) } return launchAfterConfiguration(name, runner, models[0], req) @@ -756,6 +758,7 @@ func (c *launcherClient) loadModelInventoryOnce(ctx context.Context) error { } func runIntegration(runner Runner, modelName string, args []string) error { + // TODO(parthsareen): let callers invoke runner.Run directly and remove this wrapper. return runner.Run(modelName, args) } diff --git a/cmd/launch/launch_test.go b/cmd/launch/launch_test.go index 9659856b2..1de2d58a2 100644 --- a/cmd/launch/launch_test.go +++ b/cmd/launch/launch_test.go @@ -1259,8 +1259,8 @@ func TestLaunchIntegration_ConfiguredEditorLaunchSkipsReconfigure(t *testing.T) if err := LaunchIntegration(context.Background(), IntegrationLaunchRequest{Name: "droid"}); err != nil { t.Fatalf("LaunchIntegration returned error: %v", err) } - if len(editor.edited) != 0 { - t.Fatalf("expected normal launch to skip editor rewrites, got %v", editor.edited) + if diff := compareStringSlices(editor.edited, [][]string{{"llama3.2", "qwen3:8b"}}); diff != "" { + t.Fatalf("expected normal launch to keep editor models synced (-want +got):\n%s", diff) } if editor.ranModel != "llama3.2" { t.Fatalf("expected launch to use saved primary model, got %q", editor.ranModel) @@ -1275,6 +1275,50 @@ func TestLaunchIntegration_ConfiguredEditorLaunchSkipsReconfigure(t *testing.T) } } +func TestLaunchIntegration_ConfiguredPiLaunchResyncsEditorConfig(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + + binDir := t.TempDir() + writeFakeBinary(t, binDir, "pi") + t.Setenv("PATH", binDir) + + editor := &launcherEditorRunner{} + withIntegrationOverride(t, "pi", editor) + + if err := config.SaveIntegration("pi", []string{"llama3.2", "qwen3:8b"}); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + DefaultConfirmPrompt = func(prompt string) (bool, error) { + t.Fatalf("did not expect prompt during a normal editor launch: %s", prompt) + return false, nil + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/show" { + var req apiShowRequest + _ = json.NewDecoder(r.Body).Decode(&req) + fmt.Fprintf(w, `{"model":%q}`, req.Model) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + if err := LaunchIntegration(context.Background(), IntegrationLaunchRequest{Name: "pi"}); err != nil { + t.Fatalf("LaunchIntegration returned error: %v", err) + } + if diff := compareStringSlices(editor.edited, [][]string{{"llama3.2", "qwen3:8b"}}); diff != "" { + t.Fatalf("expected configured Pi launch to sync editor models (-want +got):\n%s", diff) + } + if editor.ranModel != "llama3.2" { + t.Fatalf("expected launch to use saved primary model, got %q", editor.ranModel) + } +} + func TestLaunchIntegration_OpenclawPreservesExistingModelList(t *testing.T) { tmpDir := t.TempDir() setLaunchTestHome(t, tmpDir) @@ -1306,8 +1350,8 @@ func TestLaunchIntegration_OpenclawPreservesExistingModelList(t *testing.T) { if err := LaunchIntegration(context.Background(), IntegrationLaunchRequest{Name: "openclaw"}); err != nil { t.Fatalf("LaunchIntegration returned error: %v", err) } - if len(editor.edited) != 0 { - t.Fatalf("expected launch to preserve the existing OpenClaw config, got rewrites %v", editor.edited) + if diff := compareStringSlices(editor.edited, [][]string{{"llama3.2", "mistral"}}); diff != "" { + t.Fatalf("expected launch to keep OpenClaw config synced (-want +got):\n%s", diff) } if editor.ranModel != "llama3.2" { t.Fatalf("expected launch to use first saved model, got %q", editor.ranModel) diff --git a/cmd/launch/models.go b/cmd/launch/models.go index e78c82e02..28fde0bb5 100644 --- a/cmd/launch/models.go +++ b/cmd/launch/models.go @@ -1,6 +1,7 @@ package launch import ( + "bytes" "context" "errors" "fmt" @@ -230,11 +231,13 @@ func pullMissingModel(ctx context.Context, client *api.Client, model string) err // prepareEditorIntegration persists models and applies editor-managed config files. func prepareEditorIntegration(name string, runner Runner, editor Editor, models []string) error { - showBackupNotice := len(editor.Paths()) > 0 + paths := editor.Paths() + before := snapshotEditorFiles(paths) if err := editor.Edit(models); err != nil { return fmt.Errorf("setup failed: %w", err) } + showBackupNotice := editorFilesChanged(before, paths) if err := config.SaveIntegration(name, models); err != nil { return fmt.Errorf("failed to save: %w", err) } @@ -244,6 +247,55 @@ func prepareEditorIntegration(name string, runner Runner, editor Editor, models return nil } +type editorFileSnapshot struct { + content []byte +} + +// snapshotEditorFiles captures the current bytes for editor-managed files that +// already exist. Missing files are excluded because they cannot produce backups. +func snapshotEditorFiles(paths []string) map[string]editorFileSnapshot { + if len(paths) == 0 { + return nil + } + + snapshot := make(map[string]editorFileSnapshot, len(paths)) + for _, path := range paths { + data, err := os.ReadFile(path) + if err != nil { + continue + } + snapshot[path] = editorFileSnapshot{ + content: data, + } + } + return snapshot +} + +// editorFilesChanged reports whether any previously existing editor-managed +// file changed bytes after editing. +func editorFilesChanged(before map[string]editorFileSnapshot, paths []string) bool { + for _, path := range paths { + prev, ok := before[path] + if !ok { + // No existing file means no backup for this path. + continue + } + + after, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return true + } + continue + } + + if !bytes.Equal(prev.content, after) { + return true + } + } + return false +} + // buildModelList merges existing models with recommendations for selection UIs. func buildModelList(existing []modelInfo, preChecked []string, current string) (items []ModelItem, orderedChecked []string, existingModels, cloudModels map[string]bool) { existingModels = make(map[string]bool) diff --git a/cmd/launch/opencode_test.go b/cmd/launch/opencode_test.go index fdaea4f28..90dd85978 100644 --- a/cmd/launch/opencode_test.go +++ b/cmd/launch/opencode_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" ) @@ -288,6 +289,116 @@ func TestOpenCodeEdit(t *testing.T) { }) } +func TestOpenCodeEdit_SetsDefaultModelFromFirstSelection(t *testing.T) { + o := &OpenCode{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + configPath := filepath.Join(tmpDir, ".config", "opencode", "opencode.json") + if err := o.Edit([]string{"kimi-k2.5:cloud", "llama3.2"}); err != nil { + t.Fatal(err) + } + + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatal(err) + } + var cfg map[string]any + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatal(err) + } + if got, _ := cfg["model"].(string); got != "ollama/kimi-k2.5:cloud" { + t.Fatalf("model = %q, want %q", got, "ollama/kimi-k2.5:cloud") + } +} + +func TestOpenCodeEdit_OverridesExistingDefaultModel(t *testing.T) { + o := &OpenCode{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + configDir := filepath.Join(tmpDir, ".config", "opencode") + configPath := filepath.Join(configDir, "opencode.json") + if err := os.MkdirAll(configDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(configPath, []byte(`{ + "$schema":"https://opencode.ai/config.json", + "model":"ollama/old-model", + "provider":{"ollama":{"models":{"old-model":{"name":"old-model"}}}} + }`), 0o644); err != nil { + t.Fatal(err) + } + + if err := o.Edit([]string{"kimi-k2.5:cloud"}); err != nil { + t.Fatal(err) + } + + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatal(err) + } + var cfg map[string]any + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatal(err) + } + if got, _ := cfg["model"].(string); got != "ollama/kimi-k2.5:cloud" { + t.Fatalf("model = %q, want %q", got, "ollama/kimi-k2.5:cloud") + } +} + +func TestPrepareEditorIntegration_OpenCodeShowsBackupWarningWhenFilesChange(t *testing.T) { + o := &OpenCode{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + configDir := filepath.Join(tmpDir, ".config", "opencode") + configPath := filepath.Join(configDir, "opencode.json") + if err := os.MkdirAll(configDir, 0o755); err != nil { + t.Fatalf("failed to create config dir: %v", err) + } + if err := os.WriteFile(configPath, []byte(`{"provider":{"ollama":{"models":{"old-model":{"name":"old-model","_launch":true}}}}}`), 0o644); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + stateDir := filepath.Join(tmpDir, ".local", "state", "opencode") + statePath := filepath.Join(stateDir, "model.json") + if err := os.MkdirAll(stateDir, 0o755); err != nil { + t.Fatalf("failed to create state dir: %v", err) + } + if err := os.WriteFile(statePath, []byte(`{"recent":[{"providerID":"ollama","modelID":"old-model"}],"favorite":[],"variant":{}}`), 0o644); err != nil { + t.Fatalf("failed to seed state: %v", err) + } + + stderr := captureStderr(t, func() { + if err := prepareEditorIntegration("opencode", o, o, []string{"llama3.2"}); err != nil { + t.Fatalf("prepareEditorIntegration returned error: %v", err) + } + }) + if !strings.Contains(stderr, "OpenCode configuration has been modified. Backups are saved in") { + t.Fatalf("expected OpenCode backup warning, got stderr: %q", stderr) + } +} + +func TestPrepareEditorIntegration_OpenCodeSkipsBackupWarningWhenFilesUnchanged(t *testing.T) { + o := &OpenCode{} + tmpDir := t.TempDir() + setTestHome(t, tmpDir) + + if err := o.Edit([]string{"llama3.2"}); err != nil { + t.Fatalf("failed to seed opencode config: %v", err) + } + + stderr := captureStderr(t, func() { + if err := prepareEditorIntegration("opencode", o, o, []string{"llama3.2"}); err != nil { + t.Fatalf("prepareEditorIntegration returned error: %v", err) + } + }) + if strings.Contains(stderr, "OpenCode configuration has been modified. Backups are saved in") { + t.Fatalf("did not expect OpenCode backup warning, got stderr: %q", stderr) + } +} + func assertOpenCodeModelExists(t *testing.T, path, model string) { t.Helper() data, err := os.ReadFile(path)