diff --git a/core/gallery/models.go b/core/gallery/models.go index c2277be90093..1e5158f781aa 100644 --- a/core/gallery/models.go +++ b/core/gallery/models.go @@ -335,6 +335,12 @@ func galleryFileName(name string) string { return "._gallery_" + name + ".yaml" } +// GalleryFileName returns the on-disk filename of the gallery metadata file +// for a given installed model name (e.g. "._gallery_.yaml"). +func GalleryFileName(name string) string { + return galleryFileName(name) +} + func GetLocalModelConfiguration(basePath string, name string) (*ModelConfig, error) { name = strings.ReplaceAll(name, string(os.PathSeparator), "__") galleryFile := filepath.Join(basePath, galleryFileName(name)) diff --git a/core/http/endpoints/localai/edit_model.go b/core/http/endpoints/localai/edit_model.go index 38fdfd1d46f8..ab7476cb26d2 100644 --- a/core/http/endpoints/localai/edit_model.go +++ b/core/http/endpoints/localai/edit_model.go @@ -1,14 +1,18 @@ package localai import ( + "errors" "fmt" "io" "net/http" "net/url" "os" + "path/filepath" + "strings" "github.com/labstack/echo/v4" "github.com/mudler/LocalAI/core/config" + "github.com/mudler/LocalAI/core/gallery" httpUtils "github.com/mudler/LocalAI/core/http/middleware" "github.com/mudler/LocalAI/internal" "github.com/mudler/LocalAI/pkg/model" @@ -156,7 +160,8 @@ func EditModelEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, appC // Load the existing configuration configPath := modelConfig.GetModelConfigFile() - if err := utils.VerifyPath(configPath, appConfig.SystemState.Model.ModelsPath); err != nil { + modelsPath := appConfig.SystemState.Model.ModelsPath + if err := utils.VerifyPath(configPath, modelsPath); err != nil { response := ModelResponse{ Success: false, Error: "Model configuration not trusted: " + err.Error(), @@ -164,17 +169,89 @@ func EditModelEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, appC return c.JSON(http.StatusNotFound, response) } - // Write new content to file - if err := os.WriteFile(configPath, body, 0644); err != nil { - response := ModelResponse{ - Success: false, - Error: "Failed to write configuration file: " + err.Error(), + // Detect a rename: the URL param (old name) differs from the name field + // in the posted YAML. When that happens we must rename the on-disk file + // so that .yaml stays in sync with the internal name field — + // otherwise a subsequent config reload indexes the file under the new + // name while the old key lingers in memory, producing duplicates in the UI. + renamed := req.Name != modelName + if renamed { + if strings.ContainsRune(req.Name, os.PathSeparator) || strings.Contains(req.Name, "/") || strings.Contains(req.Name, "\\") { + response := ModelResponse{ + Success: false, + Error: "Model name must not contain path separators", + } + return c.JSON(http.StatusBadRequest, response) + } + if _, exists := cl.GetModelConfig(req.Name); exists { + response := ModelResponse{ + Success: false, + Error: fmt.Sprintf("A model named %q already exists", req.Name), + } + return c.JSON(http.StatusConflict, response) + } + newConfigPath := filepath.Join(modelsPath, req.Name+".yaml") + if err := utils.VerifyPath(newConfigPath, modelsPath); err != nil { + response := ModelResponse{ + Success: false, + Error: "New model configuration path not trusted: " + err.Error(), + } + return c.JSON(http.StatusBadRequest, response) + } + if _, err := os.Stat(newConfigPath); err == nil { + response := ModelResponse{ + Success: false, + Error: fmt.Sprintf("A configuration file for %q already exists on disk", req.Name), + } + return c.JSON(http.StatusConflict, response) + } else if !errors.Is(err, os.ErrNotExist) { + response := ModelResponse{ + Success: false, + Error: "Failed to check for existing configuration: " + err.Error(), + } + return c.JSON(http.StatusInternalServerError, response) + } + + if err := os.WriteFile(newConfigPath, body, 0644); err != nil { + response := ModelResponse{ + Success: false, + Error: "Failed to write configuration file: " + err.Error(), + } + return c.JSON(http.StatusInternalServerError, response) + } + if configPath != newConfigPath { + if err := os.Remove(configPath); err != nil && !errors.Is(err, os.ErrNotExist) { + fmt.Printf("Warning: Failed to remove old configuration file %q: %v\n", configPath, err) + } + } + + // Rename the gallery metadata file if one exists, so the delete + // flow (which looks up ._gallery_.yaml) can still find it. + oldGalleryPath := filepath.Join(modelsPath, gallery.GalleryFileName(modelName)) + newGalleryPath := filepath.Join(modelsPath, gallery.GalleryFileName(req.Name)) + if _, err := os.Stat(oldGalleryPath); err == nil { + if err := os.Rename(oldGalleryPath, newGalleryPath); err != nil { + fmt.Printf("Warning: Failed to rename gallery metadata from %q to %q: %v\n", oldGalleryPath, newGalleryPath, err) + } + } + + // Drop the stale in-memory entry before the reload so we don't + // surface both names to callers between the reload scan steps. + cl.RemoveModelConfig(modelName) + configPath = newConfigPath + } else { + // Write new content to file + if err := os.WriteFile(configPath, body, 0644); err != nil { + response := ModelResponse{ + Success: false, + Error: "Failed to write configuration file: " + err.Error(), + } + return c.JSON(http.StatusInternalServerError, response) } - return c.JSON(http.StatusInternalServerError, response) } // Reload configurations - if err := cl.LoadModelConfigsFromPath(appConfig.SystemState.Model.ModelsPath, appConfig.ToConfigLoaderOptions()...); err != nil { + if err := cl.LoadModelConfigsFromPath(modelsPath, appConfig.ToConfigLoaderOptions()...); err != nil { response := ModelResponse{ Success: false, Error: "Failed to reload configurations: " + err.Error(), @@ -183,7 +260,9 @@ func EditModelEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, appC } // Shutdown the running model to apply new configuration (e.g., context_size) - // The model will be reloaded on the next inference request + // The model will be reloaded on the next inference request. + // Shutdown uses the old name because that's what the running instance + // (if any) was started with. if err := ml.ShutdownModel(modelName); err != nil { // Log the error but don't fail the request - the config was saved successfully // The model can still be manually reloaded or restarted @@ -191,7 +270,7 @@ func EditModelEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, appC } // Preload the model - if err := cl.Preload(appConfig.SystemState.Model.ModelsPath); err != nil { + if err := cl.Preload(modelsPath); err != nil { response := ModelResponse{ Success: false, Error: "Failed to preload model: " + err.Error(), @@ -200,9 +279,13 @@ func EditModelEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, appC } // Return success response + msg := fmt.Sprintf("Model '%s' updated successfully. Model has been reloaded with new configuration.", req.Name) + if renamed { + msg = fmt.Sprintf("Model '%s' renamed to '%s' and updated successfully.", modelName, req.Name) + } response := ModelResponse{ Success: true, - Message: fmt.Sprintf("Model '%s' updated successfully. Model has been reloaded with new configuration.", modelName), + Message: msg, Filename: configPath, Config: req, } diff --git a/core/http/endpoints/localai/edit_model_test.go b/core/http/endpoints/localai/edit_model_test.go index f944c28bf349..55328dc396ee 100644 --- a/core/http/endpoints/localai/edit_model_test.go +++ b/core/http/endpoints/localai/edit_model_test.go @@ -11,7 +11,9 @@ import ( "github.com/labstack/echo/v4" "github.com/mudler/LocalAI/core/config" + "github.com/mudler/LocalAI/core/gallery" . "github.com/mudler/LocalAI/core/http/endpoints/localai" + "github.com/mudler/LocalAI/pkg/model" "github.com/mudler/LocalAI/pkg/system" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -80,5 +82,142 @@ var _ = Describe("Edit Model test", func() { Expect(string(body)).To(ContainSubstring(`"name":"foo"`)) Expect(rec.Code).To(Equal(http.StatusOK)) }) + + It("renames the config file on disk when the YAML name changes", func() { + systemState, err := system.GetSystemState( + system.WithModelPath(tempDir), + ) + Expect(err).ToNot(HaveOccurred()) + applicationConfig := config.NewApplicationConfig( + config.WithSystemState(systemState), + ) + modelConfigLoader := config.NewModelConfigLoader(systemState.Model.ModelsPath) + modelLoader := model.NewModelLoader(systemState) + + oldYAML := "name: oldname\nbackend: llama\nmodel: foo\n" + oldPath := filepath.Join(tempDir, "oldname.yaml") + Expect(os.WriteFile(oldPath, []byte(oldYAML), 0644)).To(Succeed()) + // Drop a gallery metadata file so we can check it is renamed too. + galleryOldPath := filepath.Join(tempDir, gallery.GalleryFileName("oldname")) + Expect(os.WriteFile(galleryOldPath, []byte("name: oldname\n"), 0644)).To(Succeed()) + + Expect(modelConfigLoader.LoadModelConfigsFromPath(tempDir)).To(Succeed()) + _, exists := modelConfigLoader.GetModelConfig("oldname") + Expect(exists).To(BeTrue()) + + app := echo.New() + app.POST("/models/edit/:name", EditModelEndpoint(modelConfigLoader, modelLoader, applicationConfig)) + + newYAML := "name: newname\nbackend: llama\nmodel: foo\n" + req := httptest.NewRequest("POST", "/models/edit/oldname", bytes.NewBufferString(newYAML)) + req.Header.Set("Content-Type", "application/x-yaml") + rec := httptest.NewRecorder() + app.ServeHTTP(rec, req) + + body, err := io.ReadAll(rec.Body) + Expect(err).ToNot(HaveOccurred(), string(body)) + Expect(rec.Code).To(Equal(http.StatusOK), string(body)) + + // Old file is gone, new file exists. + _, err = os.Stat(oldPath) + Expect(os.IsNotExist(err)).To(BeTrue(), "old config file should be removed") + newPath := filepath.Join(tempDir, "newname.yaml") + _, err = os.Stat(newPath) + Expect(err).ToNot(HaveOccurred(), "new config file should exist") + + // Gallery metadata followed the rename. + _, err = os.Stat(galleryOldPath) + Expect(os.IsNotExist(err)).To(BeTrue(), "old gallery metadata should be removed") + _, err = os.Stat(filepath.Join(tempDir, gallery.GalleryFileName("newname"))) + Expect(err).ToNot(HaveOccurred(), "new gallery metadata should exist") + + // In-memory config loader holds exactly one entry, keyed by the new name. + _, exists = modelConfigLoader.GetModelConfig("oldname") + Expect(exists).To(BeFalse(), "old name must not remain in config loader") + _, exists = modelConfigLoader.GetModelConfig("newname") + Expect(exists).To(BeTrue(), "new name must be present in config loader") + Expect(modelConfigLoader.GetAllModelsConfigs()).To(HaveLen(1)) + }) + + It("rejects a rename when the new name already exists", func() { + systemState, err := system.GetSystemState( + system.WithModelPath(tempDir), + ) + Expect(err).ToNot(HaveOccurred()) + applicationConfig := config.NewApplicationConfig( + config.WithSystemState(systemState), + ) + modelConfigLoader := config.NewModelConfigLoader(systemState.Model.ModelsPath) + modelLoader := model.NewModelLoader(systemState) + + Expect(os.WriteFile( + filepath.Join(tempDir, "oldname.yaml"), + []byte("name: oldname\nbackend: llama\nmodel: foo\n"), + 0644, + )).To(Succeed()) + Expect(os.WriteFile( + filepath.Join(tempDir, "newname.yaml"), + []byte("name: newname\nbackend: llama\nmodel: bar\n"), + 0644, + )).To(Succeed()) + Expect(modelConfigLoader.LoadModelConfigsFromPath(tempDir)).To(Succeed()) + + app := echo.New() + app.POST("/models/edit/:name", EditModelEndpoint(modelConfigLoader, modelLoader, applicationConfig)) + + req := httptest.NewRequest( + "POST", + "/models/edit/oldname", + bytes.NewBufferString("name: newname\nbackend: llama\nmodel: foo\n"), + ) + req.Header.Set("Content-Type", "application/x-yaml") + rec := httptest.NewRecorder() + app.ServeHTTP(rec, req) + + Expect(rec.Code).To(Equal(http.StatusConflict)) + + // Neither file should have been rewritten. + oldBody, err := os.ReadFile(filepath.Join(tempDir, "oldname.yaml")) + Expect(err).ToNot(HaveOccurred()) + Expect(string(oldBody)).To(ContainSubstring("name: oldname")) + newBody, err := os.ReadFile(filepath.Join(tempDir, "newname.yaml")) + Expect(err).ToNot(HaveOccurred()) + Expect(string(newBody)).To(ContainSubstring("model: bar")) + }) + + It("rejects a rename whose new name contains a path separator", func() { + systemState, err := system.GetSystemState( + system.WithModelPath(tempDir), + ) + Expect(err).ToNot(HaveOccurred()) + applicationConfig := config.NewApplicationConfig( + config.WithSystemState(systemState), + ) + modelConfigLoader := config.NewModelConfigLoader(systemState.Model.ModelsPath) + modelLoader := model.NewModelLoader(systemState) + + Expect(os.WriteFile( + filepath.Join(tempDir, "oldname.yaml"), + []byte("name: oldname\nbackend: llama\nmodel: foo\n"), + 0644, + )).To(Succeed()) + Expect(modelConfigLoader.LoadModelConfigsFromPath(tempDir)).To(Succeed()) + + app := echo.New() + app.POST("/models/edit/:name", EditModelEndpoint(modelConfigLoader, modelLoader, applicationConfig)) + + req := httptest.NewRequest( + "POST", + "/models/edit/oldname", + bytes.NewBufferString("name: evil/name\nbackend: llama\nmodel: foo\n"), + ) + req.Header.Set("Content-Type", "application/x-yaml") + rec := httptest.NewRecorder() + app.ServeHTTP(rec, req) + + Expect(rec.Code).To(Equal(http.StatusBadRequest)) + _, err = os.Stat(filepath.Join(tempDir, "oldname.yaml")) + Expect(err).ToNot(HaveOccurred(), "original file must not be removed") + }) }) }) diff --git a/core/http/react-ui/src/pages/ModelEditor.jsx b/core/http/react-ui/src/pages/ModelEditor.jsx index cbfc3b217886..0d0d1ccffeb1 100644 --- a/core/http/react-ui/src/pages/ModelEditor.jsx +++ b/core/http/react-ui/src/pages/ModelEditor.jsx @@ -307,8 +307,10 @@ export default function ModelEditor() { } // Refresh interactive state from saved YAML setSavedYamlText(yamlText) + let parsedName = null try { const parsed = YAML.parse(yamlText) + parsedName = parsed?.name ?? null const flat = flattenConfig(parsed || {}) setValues(flat) setInitialValues(structuredClone(flat)) @@ -316,6 +318,12 @@ export default function ModelEditor() { } catch { /* ignore parse failure */ } setTabSwitchWarning(false) addToast('Config saved', 'success') + // When the model was renamed via the YAML `name:` field, the current + // editor URL points at a name that no longer exists on the backend. + // Redirect so refreshes and subsequent saves hit the new name. + if (parsedName && parsedName !== name) { + navigate(`/app/model-editor/${encodeURIComponent(parsedName)}`, { replace: true }) + } } } catch (err) { addToast(`Save failed: ${err.message}`, 'error')