mirror of
https://github.com/ollama/ollama.git
synced 2026-04-17 21:54:08 +02:00
launch: remove banner and warn only when backup-relevant configs change (#15124)
This commit is contained in:
committed by
ParthSareen
parent
04e41ddcfb
commit
cdd0bc48a3
@@ -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" {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user