mirror of
https://github.com/mudler/LocalAI.git
synced 2026-04-18 13:58:07 -04:00
fix(ui): rename model config files on save to prevent duplicates (#9388)
Editing a model's YAML and changing the `name:` field previously wrote the new body to the original `<oldName>.yaml`. On reload the config loader indexed that file under the new name while the old key lingered in memory, producing two entries in the system UI that shared a single underlying file — deleting either removed both. Detect the rename in EditModelEndpoint and rename the on-disk `<name>.yaml` and `._gallery_<name>.yaml` to match, drop the stale in-memory key before the reload, and redirect the editor URL in the React UI so it tracks the new name. Reject conflicts (409) and names containing path separators (400). Fixes #9294
This commit is contained in:
committed by
GitHub
parent
5837b14888
commit
7c5d6162f7
@@ -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_<name>.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))
|
||||
|
||||
@@ -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 <name>.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_<name>.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,
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user