fix(gallery): key meta pin recall on the installed name and detach resolved entries

Six review findings on the meta-entry install path. Pin recall was keyed on the gallery entry name while applyModel writes the record under the install name (req.Name when supplied), so a meta installed under a custom name with a pin lost that pin on reinstall and was silently re-resolved onto a different variant, possibly swapping its backend. Compute the install name with applyModel's own precedence before the recall. ResolveMetaModel returned a shallow struct copy, so the resolved entry's Overrides aliased the gallery entry's map and the install path's in-place mergo merge wrote the caller's request into the shared catalog. Detach Overrides, ConfigFile, AdditionalFiles, URLs and Tags. Not exploitable today only because this path re-unmarshals the gallery per call, which is a property nobody should have to rely on. Also: overlay the meta's name onto the persisted config for meta installs so the gallery file no longer records the variant's name; move the pinned-VRAM warning below the variant validation so a pin naming a nonexistent entry does not warn about VRAM before failing for an unrelated reason; and stop seeding config.URLs in the config_file branch, which duplicated every declared URL. Add seven network-free specs driving InstallModelFromGallery with a meta entry: variant payload wins over the meta's legacy url fallback, the resolution record round-trips to disk, a pin is recorded and honored on reinstall including under a custom install name, and the resolved entry does not alias the gallery's maps. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-09-19 03:32:29 -04:00 · 2026-07-18 10:36:04 +00:00
1 parent 49fa504160
commit 47f37fa80d
@@ -1,11 +1,17 @@
 package gallery_test

 import (
+	"context"
+	"os"
+	"path/filepath"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"gopkg.in/yaml.v3"

+	"github.com/mudler/LocalAI/core/config"
 	"github.com/mudler/LocalAI/core/gallery"
+	"github.com/mudler/LocalAI/pkg/system"
 )

 var _ = Describe("GalleryModel meta entries", func() {
@@ -93,6 +99,28 @@ var _ = Describe("ResolveMetaModel", func() {
 		Expect(resolved.URL).To(Equal("file://gguf.yaml"))
 	})

+	It("detaches the resolved entry from the gallery's own maps and slices", func() {
+		models[0].Overrides = map[string]any{"f16": true}
+		models[0].AdditionalFiles = []gallery.File{{Filename: "a.bin"}}
+
+		resolved, _, err := gallery.ResolveMetaModel(models, meta, gallery.ResolveEnv{Capability: "nvidia", VRAM: gib(24)}, "")
+		Expect(err).ToNot(HaveOccurred())
+
+		// The install path merges the caller's request overrides into this map in
+		// place, so aliasing the gallery's map would write one caller's request
+		// into the shared catalog and leak it into every later install.
+		resolved.Overrides["f16"] = false
+		resolved.Overrides["threads"] = 4
+		Expect(models[0].Overrides).To(HaveKeyWithValue("f16", true))
+		Expect(models[0].Overrides).ToNot(HaveKey("threads"))
+
+		resolved.AdditionalFiles[0].Filename = "mutated.bin"
+		Expect(models[0].AdditionalFiles[0].Filename).To(Equal("a.bin"))
+
+		resolved.Tags = append(resolved.Tags, "extra")
+		Expect(meta.Tags).To(ConsistOf("llm"))
+	})
+
 	It("errors when a candidate references a missing entry", func() {
 		meta.Candidates = []gallery.Candidate{{Model: "does-not-exist"}}
 		_, _, err := gallery.ResolveMetaModel(models, meta, gallery.ResolveEnv{Capability: "default", VRAM: gib(8)}, "")
@@ -115,3 +143,166 @@ var _ = Describe("ResolveMetaModel", func() {
 		Expect(err).To(MatchError(gallery.ErrPinNotFound))
 	})
 })
+
+var _ = Describe("InstallModelFromGallery with meta entries", func() {
+	var tempdir string
+	var galleries []config.Gallery
+	var systemState *system.SystemState
+
+	// The variants are described with an inline config_file rather than a URL so
+	// the whole install runs off the local filesystem with no network access.
+	// The meta entry keeps a url as well, because a real meta entry carries one
+	// as a fallback for older LocalAI releases that do not understand candidates,
+	// and installing that fallback instead of a variant is exactly the regression
+	// these specs guard against.
+	newGallery := func(meta gallery.GalleryModel, variants ...gallery.GalleryModel) {
+		fallback := gallery.ModelConfig{
+			Name:        "legacy-fallback",
+			Description: "legacy fallback payload",
+			ConfigFile:  "backend: fallback-backend\n",
+		}
+		fallbackYAML, err := yaml.Marshal(fallback)
+		Expect(err).ToNot(HaveOccurred())
+		fallbackPath := filepath.Join(tempdir, "fallback.yaml")
+		Expect(os.WriteFile(fallbackPath, fallbackYAML, 0600)).To(Succeed())
+
+		meta.URL = "file://" + fallbackPath
+		entries := append([]gallery.GalleryModel{meta}, variants...)
+
+		out, err := yaml.Marshal(entries)
+		Expect(err).ToNot(HaveOccurred())
+		galleryPath := filepath.Join(tempdir, "gallery.yaml")
+		Expect(os.WriteFile(galleryPath, out, 0600)).To(Succeed())
+
+		galleries = []config.Gallery{{Name: "test", URL: "file://" + galleryPath}}
+	}
+
+	variant := func(name, backend string) gallery.GalleryModel {
+		m := gallery.GalleryModel{ConfigFile: map[string]any{"backend": backend}}
+		m.Name = name
+		m.Description = "variant " + name
+		return m
+	}
+
+	metaEntry := func(name string, candidates ...string) gallery.GalleryModel {
+		m := gallery.GalleryModel{}
+		m.Name = name
+		m.Description = "the meta entry"
+		m.Icon = "meta.png"
+		for _, c := range candidates {
+			m.Candidates = append(m.Candidates, gallery.Candidate{Model: c})
+		}
+		return m
+	}
+
+	install := func(name string, req gallery.GalleryModel, options ...gallery.InstallOption) error {
+		return gallery.InstallModelFromGallery(
+			context.TODO(), galleries, []config.Gallery{}, systemState, nil,
+			name, req, func(string, string, string, float64) {}, false, false, false, options...)
+	}
+
+	installedBackend := func(name string) string {
+		dat, err := os.ReadFile(filepath.Join(tempdir, name+".yaml"))
+		Expect(err).ToNot(HaveOccurred())
+		content := map[string]any{}
+		Expect(yaml.Unmarshal(dat, &content)).To(Succeed())
+		return content["backend"].(string)
+	}
+
+	BeforeEach(func() {
+		var err error
+		tempdir, err = os.MkdirTemp("", "meta-install")
+		Expect(err).ToNot(HaveOccurred())
+		DeferCleanup(func() { Expect(os.RemoveAll(tempdir)).To(Succeed()) })
+
+		systemState, err = system.GetSystemState(system.WithModelPath(tempdir))
+		Expect(err).ToNot(HaveOccurred())
+
+		newGallery(
+			metaEntry("qwen3-8b", "qwen3-8b-variant-a", "qwen3-8b-variant-b"),
+			variant("qwen3-8b-variant-a", "variant-a-backend"),
+			variant("qwen3-8b-variant-b", "variant-b-backend"),
+		)
+	})
+
+	It("installs the resolved variant's payload, not the meta's url fallback", func() {
+		Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed())
+
+		// If meta-ness stopped winning over the url, this would be
+		// "fallback-backend" and every meta entry would silently install the
+		// legacy payload instead of a hardware-appropriate variant.
+		Expect(installedBackend("qwen3-8b")).To(Equal("variant-a-backend"))
+	})
+
+	It("round-trips the resolution record to disk under the meta's name", func() {
+		Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed())
+
+		record, err := gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(record.MetaName).To(Equal("qwen3-8b"))
+		Expect(record.ResolvedVariant).To(Equal("qwen3-8b-variant-a"))
+		Expect(record.PinnedVariant).To(BeEmpty())
+		Expect(record.Name).To(Equal("qwen3-8b"))
+		Expect(record.Description).To(Equal("the meta entry"))
+	})
+
+	It("records a pin and honors it on a plain reinstall", func() {
+		Expect(install("qwen3-8b", gallery.GalleryModel{}, gallery.WithVariant("qwen3-8b-variant-b"))).To(Succeed())
+
+		record, err := gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(record.PinnedVariant).To(Equal("qwen3-8b-variant-b"))
+		Expect(record.ResolvedVariant).To(Equal("qwen3-8b-variant-b"))
+		Expect(installedBackend("qwen3-8b")).To(Equal("variant-b-backend"))
+
+		// No WithVariant this time: hardware resolution would pick variant-a, so
+		// only the recalled pin can keep this on variant-b.
+		Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed())
+		Expect(installedBackend("qwen3-8b")).To(Equal("variant-b-backend"))
+
+		record, err = gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(record.PinnedVariant).To(Equal("qwen3-8b-variant-b"))
+	})
+
+	It("honors a pin recorded under a custom install name", func() {
+		req := gallery.GalleryModel{}
+		req.Name = "prod-llm"
+
+		Expect(install("qwen3-8b", req, gallery.WithVariant("qwen3-8b-variant-b"))).To(Succeed())
+
+		// The pin is written under the installed name, so the recall must read it
+		// back under that name too and not under the gallery entry's name.
+		record, err := gallery.GetLocalModelConfiguration(tempdir, "prod-llm")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(record.PinnedVariant).To(Equal("qwen3-8b-variant-b"))
+		Expect(record.MetaName).To(Equal("qwen3-8b"))
+		Expect(installedBackend("prod-llm")).To(Equal("variant-b-backend"))
+
+		Expect(install("qwen3-8b", req)).To(Succeed())
+		Expect(installedBackend("prod-llm")).To(Equal("variant-b-backend"))
+	})
+
+	It("does not write the caller's overrides back into the gallery entry", func() {
+		req := gallery.GalleryModel{Overrides: map[string]any{"f16": true}}
+		Expect(install("qwen3-8b", req)).To(Succeed())
+
+		models, err := gallery.AvailableGalleryModels(galleries, systemState)
+		Expect(err).ToNot(HaveOccurred())
+		entry := gallery.FindGalleryElement(models, "qwen3-8b-variant-a")
+		Expect(entry).ToNot(BeNil())
+		Expect(entry.Overrides).ToNot(HaveKey("f16"))
+	})
+
+	It("writes each declared url only once into the persisted gallery file", func() {
+		meta := metaEntry("qwen3-8b", "qwen3-8b-variant-a")
+		meta.URLs = []string{"https://example.invalid/qwen3"}
+		newGallery(meta, variant("qwen3-8b-variant-a", "variant-a-backend"))
+
+		Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed())
+
+		record, err := gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(record.URLs).To(ConsistOf("https://example.invalid/qwen3"))
+	})
+})
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -94,16 +95,6 @@ func ResolveMetaModel(models []*GalleryModel, meta *GalleryModel, env ResolveEnv
 		return nil, Candidate{}, fmt.Errorf("resolving variant for model %q: %w", meta.Name, err)
 	}

-	// A pin is an operator override and deliberately bypasses the hardware
-	// checks, but a silent bypass makes a later out-of-memory failure
-	// impossible to trace back to the pin, so it is recorded loudly here.
-	if pin != "" {
-		if floor, declared, verr := candidate.EffectiveMinVRAM(); verr == nil && declared && env.VRAM < floor {
-			xlog.Warn("Pinned model variant declares more VRAM than this system reports; installing anyway because the pin overrides hardware resolution",
-				"model", meta.Name, "variant", candidate.Model, "required_vram", floor, "available_vram", env.VRAM)
-		}
-	}
-
 	concrete := FindGalleryElement(models, candidate.Model)
 	if concrete == nil {
 		return nil, Candidate{}, fmt.Errorf("model %q references variant %q which does not exist in any configured gallery", meta.Name, candidate.Model)
@@ -112,15 +103,38 @@ func ResolveMetaModel(models []*GalleryModel, meta *GalleryModel, env ResolveEnv
 		return nil, Candidate{}, fmt.Errorf("model %q references variant %q which is itself a meta entry; meta entries may not nest", meta.Name, candidate.Model)
 	}

+	// A pin is an operator override and deliberately bypasses the hardware
+	// checks, but a silent bypass makes a later out-of-memory failure
+	// impossible to trace back to the pin, so it is recorded loudly here.
+	// It is warned about only once the pin is known to name a real, installable
+	// entry, otherwise a pin naming a listed-but-nonexistent variant would warn
+	// about VRAM and then fail for an entirely unrelated reason.
+	if pin != "" {
+		if floor, declared, verr := candidate.EffectiveMinVRAM(); verr == nil && declared && env.VRAM < floor {
+			xlog.Warn("Pinned model variant declares more VRAM than this system reports; installing anyway because the pin overrides hardware resolution",
+				"model", meta.Name, "variant", candidate.Model, "required_vram", floor, "available_vram", env.VRAM)
+		}
+	}
+
 	resolved := *concrete
 	resolved.Name = meta.Name
 	resolved.Description = meta.Description
 	resolved.Icon = meta.Icon
 	resolved.License = meta.License
-	resolved.URLs = meta.URLs
-	resolved.Tags = meta.Tags
 	resolved.Candidates = nil

+	// The struct copy above is shallow, so every reference-typed field still
+	// aliases the gallery's own entries. The install path mutates Overrides in
+	// place (mergo merges the caller's request overrides into it) and appends to
+	// the URL and tag slices, which would write the caller's request into the
+	// gallery catalog itself and leak between installs the moment this path
+	// reads from a cached, long-lived gallery listing. Detach them here.
+	resolved.Overrides = maps.Clone(concrete.Overrides)
+	resolved.ConfigFile = maps.Clone(concrete.ConfigFile)
+	resolved.AdditionalFiles = slices.Clone(concrete.AdditionalFiles)
+	resolved.URLs = slices.Clone(meta.URLs)
+	resolved.Tags = slices.Clone(meta.Tags)
+
 	return &resolved, candidate, nil
 }

@@ -157,9 +171,11 @@ func InstallModelFromGallery(
 				ConfigFile:  string(reYamlConfig),
 				Description: model.Description,
 				License:     model.License,
-				URLs:        model.URLs,
 				Name:        model.Name,
 				Files:       make([]File, 0), // Real values get added below, must be blank
+				// URLs are deliberately not seeded here: they are appended once
+				// below for both branches, and seeding them too would write every
+				// URL twice into the persisted gallery file.
 				// Prompt Template Skipped for now - I expect in this mode that they will be delivered as files.
 			}
 		} else {
@@ -170,6 +186,10 @@ func InstallModelFromGallery(
 			config.MetaName = record.MetaName
 			config.ResolvedVariant = record.ResolvedVariant
 			config.PinnedVariant = record.PinnedVariant
+			// The variant's own name would otherwise be persisted here, which
+			// contradicts the whole point of a meta entry: the model is known by
+			// the meta's stable name regardless of which variant backs it.
+			config.Name = record.MetaName
 		}

 		installName := model.Name
@@ -227,8 +247,19 @@ func InstallModelFromGallery(
 	// A previously recorded pin survives reinstalls and upgrades, so a user
 	// who deliberately chose a variant is not silently re-resolved onto a
 	// different one by a hardware or gallery change.
+	//
+	// The record is keyed by the name the model was installed under, not by the
+	// gallery entry name: applyModel writes it to ._gallery_<installName>.yaml,
+	// where installName is req.Name whenever the caller supplied one. Reading it
+	// back under the meta's own name would miss the record for every custom-named
+	// install and silently re-resolve a deliberately pinned model onto a
+	// different variant, possibly swapping its backend.
 	if pin == "" {
-		if previous, err := GetLocalModelConfiguration(systemState.Model.ModelsPath, model.Name); err == nil && previous != nil {
+		installName := model.Name
+		if req.Name != "" {
+			installName = req.Name
+		}
+		if previous, err := GetLocalModelConfiguration(systemState.Model.ModelsPath, installName); err == nil && previous != nil {
 			pin = previous.PinnedVariant
 		}
 	}