diff --git a/cmd/mavend/mail.go b/cmd/mavend/mail.go index 2e6e796..ebde48a 100644 --- a/cmd/mavend/mail.go +++ b/cmd/mavend/mail.go @@ -27,7 +27,6 @@ import ( "github.com/kami/maven/internal/config" "github.com/kami/maven/internal/email" "github.com/kami/maven/internal/ipc" - "github.com/kami/maven/internal/llm" "github.com/kami/maven/internal/phraser" "github.com/kami/maven/internal/store" ) @@ -66,7 +65,7 @@ func newMailIntake(st *store.Store, phr phraser.Phraser, cfg *config.Config) *ma if timeout <= 0 { timeout = config.DefaultEmailTimeout } - ex := email.NewExtractor(llm.New(lp.BaseURL(), timeout), cfg.Email.MaxTasks, contextBlockFn(cfg, time.Now)) + ex := email.NewExtractor(llmClientFor(lp, timeout), cfg.Email.MaxTasks, contextBlockFn(cfg, time.Now)) log.Printf("mail intake: enabled (max %d candidates per message, timeout %s)", cfg.Email.MaxTasks, timeout) return &mailIntake{st: st, ex: ex, timeout: timeout, now: time.Now} } diff --git a/cmd/mavend/main.go b/cmd/mavend/main.go index 0c10d80..e4fffa2 100644 --- a/cmd/mavend/main.go +++ b/cmd/mavend/main.go @@ -329,6 +329,7 @@ func run(args []string) error { // ipc.MethodIngestMail reports ErrUnknownMethod. if !locked { wireMailIntake(srv, st, phr, cfg) + wireModelSwap(srv, phr, cfg) } // WrapKeyFn — wraps the env key with a passkey credential public key and @@ -471,6 +472,7 @@ func run(args []string) error { srv.SetAPI(newAPI) srv.Check = (&auth.Gate{Enrollment: auth.NewFloorEnrollment(), Session: passkeySess}).Check wireMailIntake(srv, st, phr, cfg) + wireModelSwap(srv, phr, cfg) // Start voice server. if voiceW != nil { diff --git a/cmd/mavend/memoryeval.go b/cmd/mavend/memoryeval.go index 9f0c641..2faea11 100644 --- a/cmd/mavend/memoryeval.go +++ b/cmd/mavend/memoryeval.go @@ -16,7 +16,6 @@ import ( "time" "github.com/kami/maven/internal/config" - "github.com/kami/maven/internal/llm" "github.com/kami/maven/internal/memeval" "github.com/kami/maven/internal/phraser" "github.com/kami/maven/internal/store" @@ -50,7 +49,7 @@ func newMemoryEvalWorker(st *store.Store, phr phraser.Phraser, cfg *config.Confi } // A generous per-request timeout: this is a long prompt to a Thinking model // and nobody is waiting on the answer. - client := llm.New(lp.BaseURL(), 5*time.Minute) + client := llmClientFor(lp, 5*time.Minute) ev := memeval.NewEvaluator(st, st, client, memeval.Config{ MaxItems: cfg.MemoryEval.MaxItems, MinConfidence: cfg.MemoryEval.MinConfidence, diff --git a/cmd/mavend/modelswap.go b/cmd/mavend/modelswap.go new file mode 100644 index 0000000..fae8cb7 --- /dev/null +++ b/cmd/mavend/modelswap.go @@ -0,0 +1,113 @@ +package main + +import ( + "context" + "fmt" + "log" + "path/filepath" + "time" + + "github.com/kami/maven/internal/config" + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/llm" + "github.com/kami/maven/internal/phraser" +) + +// Swapping the resident model while the daemon runs (Vikunja #250). +// +// Off unless configured: with no phraser.swap_models allowlist the two IPC +// methods are never wired, so they answer ErrUnknownMethod. When it is wired the +// swap method is AuthStepUp (internal/auth), which means an authed human surface +// only — there is no act, no intent and no timer that reaches it. The daemon +// never decides to change its own brain. +// +// The allowlist is exact-match against paths a human wrote in mavend.json. The +// request carries a path and llama-server is started with it as `-m`, so +// anything looser would turn "swap the model" into "load any file on my disk". +func wireModelSwap(srv *ipc.Server, phr phraser.Phraser, cfg *config.Config) { + if cfg.Phraser == nil || len(cfg.Phraser.SwapModels) == 0 { + return + } + lp, ok := phr.(*phraser.LLMPhraser) + if !ok { + log.Printf("model swap: phraser.swap_models is set but there is no llama-server phraser — swap disabled") + return + } + allowed := map[string]bool{} + for _, m := range cfg.Phraser.SwapModels { + allowed[filepath.Clean(m)] = true + } + // The configured model is always swappable back to, listed or not: the way + // out of a bad swap must not depend on remembering to allowlist the model + // you are already running. + allowed[filepath.Clean(cfg.Phraser.ModelPath)] = true + + srv.SwapModelFn = func(ctx context.Context, req ipc.SwapModelReq) (ipc.SwapModelResp, error) { + path := filepath.Clean(req.ModelPath) + if !allowed[path] { + log.Printf("model swap: REFUSED %q — not in phraser.swap_models", req.ModelPath) + return ipc.SwapModelResp{}, fmt.Errorf("%w: %q is not in phraser.swap_models", ipc.ErrForbidden, req.ModelPath) + } + res, err := lp.Swap(ctx, phraser.SwapSpec{ + ModelPath: path, + NGpuLayers: req.NGpuLayers, + NCtx: req.NCtx, + }) + resp := ipc.SwapModelResp{ + Model: res.Model, + ModelPath: res.ModelPath, + BaseURL: res.BaseURL, + RolledBack: res.RolledBack, + TookMs: res.Took.Milliseconds(), + } + if err != nil { + // A rolled-back swap is a failure that left a working daemon behind. + // Both halves matter to the caller, so the response is filled in even + // though the error is returned. + log.Printf("model swap: %v", err) + return resp, err + } + return resp, nil + } + + srv.ModelStatusFn = func(ctx context.Context) (ipc.ModelStatusResp, error) { + path, ngl, nctx := lp.LiveModel() + base := lp.BaseURL() + resp := ipc.ModelStatusResp{ + ModelPath: path, + BaseURL: base, + NGpuLayers: ngl, + NCtx: nctx, + Swappable: cfg.Phraser.SwapModels, + } + if base == "" { + resp.Model = llm.UnknownModel + return resp, nil + } + id, err := llm.ModelID(ctx, base) + if err != nil { + // Report the honest "I could not confirm it" rather than echoing the + // configured filename as if the server had said it. + resp.Model = llm.UnknownModel + return resp, nil + } + resp.Model = id + return resp, nil + } + + log.Printf("model swap: enabled, %d allowlisted model(s) — step-up required", len(cfg.Phraser.SwapModels)) +} + +// llmClientFor builds a completion client on the phraser's llama-server and +// keeps it pointed at the right one across a model swap. +// +// Without the OnSwap registration every holder of a base URL — the LLM router, +// the replier, the mail extractor, the memory evaluator — would keep talking to +// the port of a server that no longer exists, and the daemon would degrade to +// the classifier permanently after the first swap. The client is re-pointed, not +// rebuilt, so nothing that holds it has to know a swap happened. +func llmClientFor(lp *phraser.LLMPhraser, timeout time.Duration) *llm.Client { + c := llm.New(lp.BaseURL(), timeout) + lp.OnSwap(func(base string) { c.SetBaseURL(base) }) + return c +} diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index 7167511..06fa81a 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -148,7 +148,9 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem // The replier uses the same llama-server as the phraser. var llmClient *llm.Client if lp, ok := phr.(*phraser.LLMPhraser); ok { - llmClient = llm.New(lp.BaseURL(), 60*time.Second) + // llmClientFor, not llm.New: this client must follow the phraser onto + // the new llama-server when the resident model is swapped (Vikunja #250). + llmClient = llmClientFor(lp, 60*time.Second) } // ----- router (the cascade; floor examples seed the classifier) ----- // The act matcher's allowlist is exactly the enabled tool names — the diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index b88bb06..4491084 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -124,6 +124,7 @@ var sidebarSections = []struct { Label: "Settings", Pages: []struct{ Label, URL, Key string }{ {Label: "Tools", URL: "/tools", Key: "tools"}, + {Label: "Model", URL: "/models", Key: "models"}, {Label: "Passkey", URL: "/auth/passkey", Key: "passkey"}, }, }, @@ -191,6 +192,8 @@ func pageIcon(key string) string { return `` case "tools": return `` + case "models": + return `` case "passkey": return `` default: @@ -225,6 +228,8 @@ func pageTitle(key string) string { return "Ecosystem" case "tools": return "Tools" + case "models": + return "Resident Model" case "passkey": return "Passkey" default: @@ -479,6 +484,12 @@ func main() { mux.HandleFunc("/routines", func(w http.ResponseWriter, r *http.Request) { handleRoutines(w, r, core, stepUpSession, *requireStepUp) }) + // /models — the resident-model surface (Vikunja #250). Same step-up gate as + // /tools, and for a comparable reason: which model is loaded decides how every + // utterance is routed and how every reply is worded. GET is read-only. + mux.HandleFunc("/models", func(w http.ResponseWriter, r *http.Request) { + handleModels(w, r, core, stepUpSession, *requireStepUp) + }) // State-changing routes on this server, and their gate (Vikunja #317): // diff --git a/cmd/mavweb/models.go b/cmd/mavweb/models.go new file mode 100644 index 0000000..11c4d41 --- /dev/null +++ b/cmd/mavweb/models.go @@ -0,0 +1,145 @@ +package main + +import ( + "context" + "errors" + "html/template" + "log" + "net/http" + "strconv" + "strings" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/webauthn" +) + +// The resident-model surface (Vikunja #250). +// +// GET shows which model llama-server actually has loaded and which files the +// daemon is configured to allow. POST swaps to one of them, behind the same +// step-up gate as POST /tools: the loaded model decides how every utterance is +// routed and how every reply is worded, so it is an owner action. +// +// There is nothing on this page Maven can press. The swap is an IPC method rated +// AuthStepUp in internal/auth, unreachable from an act, an intent or a timer. + +// modelController — the two non-CoreAPI methods this page needs. *ipc.Client +// satisfies it; a core without a swap allowlist answers ErrUnknownMethod, which +// the page renders as "not configured" rather than an error. +type modelController interface { + ModelStatus(ctx context.Context) (ipc.ModelStatusResp, error) + SwapModel(ctx context.Context, req ipc.SwapModelReq) (ipc.SwapModelResp, error) +} + +var modelsTmpl = template.Must(template.New("models").Funcs(shellFuncs()).Parse(shellTopHTML + modelsHTML + shellBottomHTML)) + +const modelsHTML = `{{template "shellTop" "models"}} +

Resident model

+

swapping requires step-up — assert a passkey first. The old model is unloaded before the new one is loaded (one model fits the iGPU at a time), so turns during the load are refused and fall back to the classifier.

+{{if .Msg}}
{{.Msg}}
{{end}} +{{if .Err}}
{{.Err}}
{{end}} +{{if .Off}} +
+

swap not configured

+

this core has no phraser.swap_models allowlist, so there is nothing to swap to. Add the gguf paths you allow to deploy/mavend.json and restart once.

+
+{{else}} +
+

loaded now

+
+ + + + + +
model{{.Status.Model}}
file{{.Status.ModelPath}}
server{{.Status.BaseURL}}
n_ctx{{.Status.NCtx}}
n_gpu_layers{{.Status.NGpuLayers}}
+

the model name is what llama-server reports for itself, not what the config says it should be.

+
+
+

allowed models {{len .Status.Swappable}}

+{{if .Status.Swappable}}
+{{range .Status.Swappable}} +{{end}} +
file
{{.}}
+ +
+{{else}}
no models allowlisted
{{end}} +
+{{end}} +{{template "shellBottom"}}` + +type modelsPage struct { + Msg string + Err string + Off bool + Status ipc.ModelStatusResp +} + +// handleModels renders the model surface (GET) and applies a swap (POST). +// +// A failed swap is reported as a failure with the model that is still serving +// named, because that is the state the operator needs: the daemon rolled back +// and is answering turns, it just is not answering them with what he asked for. +func handleModels(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, session *webauthn.PasskeySession, requireStepUp bool) { + if core == nil { + http.Error(w, "models disabled (no -core)", http.StatusServiceUnavailable) + return + } + mc, ok := core.(modelController) + if !ok { + http.Error(w, "models unavailable: core connection does not support model swap", http.StatusServiceUnavailable) + return + } + ctx := r.Context() + page := modelsPage{} + + if r.Method == http.MethodPost { + if !stepUpOK(session, requireStepUp) { + http.Error(w, "step-up required: assert a passkey first", http.StatusForbidden) + return + } + path := strings.TrimSpace(r.FormValue("model_path")) + if path == "" { + http.Error(w, "model_path required", http.StatusBadRequest) + return + } + req := ipc.SwapModelReq{ModelPath: path} + if v, err := strconv.Atoi(r.FormValue("n_ctx")); err == nil { + req.NCtx = v + } + res, err := mc.SwapModel(ctx, req) + switch { + case err == nil: + page.Msg = "loaded " + res.Model + " (" + strconv.FormatInt(res.TookMs, 10) + "ms)" + log.Printf("models: swapped to %s (%s) in %dms", res.ModelPath, res.Model, res.TookMs) + case errors.Is(err, ipc.ErrForbidden): + http.Error(w, "refused: that model is not in phraser.swap_models, or step-up was not asserted", http.StatusForbidden) + return + case errors.Is(err, ipc.ErrUnknownMethod): + http.Error(w, "swap not configured on this core", http.StatusServiceUnavailable) + return + case res.RolledBack: + page.Err = "swap failed, rolled back to " + res.Model + " — she is still answering, with the old model" + log.Printf("models: swap to %s failed, rolled back: %v", path, err) + default: + page.Err = "swap failed: " + err.Error() + log.Printf("models: swap to %s failed: %v", path, err) + } + } + + st, err := mc.ModelStatus(ctx) + if err != nil { + if errors.Is(err, ipc.ErrUnknownMethod) { + page.Off = true + } else { + log.Printf("models: status: %v", err) + http.Error(w, "core read failed", http.StatusBadGateway) + return + } + } + page.Status = st + w.Header().Set("Content-Type", "text/html; charset=utf-8") + if err := modelsTmpl.Execute(w, page); err != nil { + log.Printf("models render: %v", err) + } +} diff --git a/cmd/mavweb/models_test.go b/cmd/mavweb/models_test.go new file mode 100644 index 0000000..1115e9e --- /dev/null +++ b/cmd/mavweb/models_test.go @@ -0,0 +1,150 @@ +package main + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/webauthn" +) + +// fakeModelCore is a core that supports the two model methods. It records what +// the page asked for, so the tests can assert the gate rather than the HTML. +type fakeModelCore struct { + ipc.UnimplementedCoreAPI + + status ipc.ModelStatusResp + statusErr error + + swapResp ipc.SwapModelResp + swapErr error + swapped []ipc.SwapModelReq +} + +func (f *fakeModelCore) ModelStatus(ctx context.Context) (ipc.ModelStatusResp, error) { + return f.status, f.statusErr +} + +func (f *fakeModelCore) SwapModel(ctx context.Context, req ipc.SwapModelReq) (ipc.SwapModelResp, error) { + f.swapped = append(f.swapped, req) + return f.swapResp, f.swapErr +} + +func modelsGET(t *testing.T, core ipc.CoreAPI) *httptest.ResponseRecorder { + t.Helper() + w := httptest.NewRecorder() + handleModels(w, httptest.NewRequest(http.MethodGet, "/models", nil), core, nil, false) + return w +} + +func modelsPOST(t *testing.T, core ipc.CoreAPI, session *webauthn.PasskeySession, requireStepUp bool, path string) *httptest.ResponseRecorder { + t.Helper() + r := httptest.NewRequest(http.MethodPost, "/models", strings.NewReader("model_path="+path)) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + handleModels(w, r, core, session, requireStepUp) + return w +} + +func TestModels_GETShowsTheLoadedModelAndTheAllowlist(t *testing.T) { + core := &fakeModelCore{status: ipc.ModelStatusResp{ + Model: "Qwen3-1.7B-UD-Q4_K_XL", + ModelPath: "/opt/maven/models/llm/qwen3.gguf", + BaseURL: "http://127.0.0.1:18099", + NCtx: 4096, + Swappable: []string{"/opt/maven/models/llm/qwen3.gguf", "/opt/maven/models/llm/qwen3-cpt.gguf"}, + }} + w := modelsGET(t, core) + if w.Code != http.StatusOK { + t.Fatalf("GET /models = %d; want 200", w.Code) + } + body := w.Body.String() + for _, want := range []string{"Qwen3-1.7B-UD-Q4_K_XL", "qwen3-cpt.gguf", "4096"} { + if !strings.Contains(body, want) { + t.Errorf("page does not mention %q", want) + } + } + if len(core.swapped) != 0 { + t.Errorf("a GET swapped the model: %v", core.swapped) + } +} + +func TestModels_POSTRequiresStepUpWhenFailingClosed(t *testing.T) { + // No WebAuthn configured (nil session) + -require-stepup ⇒ deny, exactly + // like POST /tools. Nothing reaches core. + core := &fakeModelCore{} + w := modelsPOST(t, core, nil, true, "/opt/maven/models/llm/qwen3.gguf") + if w.Code != http.StatusForbidden { + t.Fatalf("POST /models without assertable step-up = %d; want 403", w.Code) + } + if len(core.swapped) != 0 { + t.Fatalf("a denied POST still called SwapModel: %v", core.swapped) + } +} + +func TestModels_POSTSwapsAndReportsTheModelThatAnswered(t *testing.T) { + core := &fakeModelCore{ + swapResp: ipc.SwapModelResp{Model: "qwen3-cpt", ModelPath: "/m/cpt.gguf", TookMs: 4200}, + status: ipc.ModelStatusResp{Model: "qwen3-cpt", ModelPath: "/m/cpt.gguf"}, + } + w := modelsPOST(t, core, nil, false, "/m/cpt.gguf") + if w.Code != http.StatusOK { + t.Fatalf("POST /models = %d; want 200", w.Code) + } + if len(core.swapped) != 1 || core.swapped[0].ModelPath != "/m/cpt.gguf" { + t.Fatalf("SwapModel calls = %v; want one for /m/cpt.gguf", core.swapped) + } + if !strings.Contains(w.Body.String(), "loaded qwen3-cpt") { + t.Errorf("page does not report which model was loaded:\n%s", w.Body.String()) + } +} + +func TestModels_RolledBackSwapSaysSheIsStillAnswering(t *testing.T) { + core := &fakeModelCore{ + swapResp: ipc.SwapModelResp{Model: "qwen3", ModelPath: "/m/old.gguf", RolledBack: true}, + swapErr: errBrokenModel{}, + status: ipc.ModelStatusResp{Model: "qwen3", ModelPath: "/m/old.gguf"}, + } + w := modelsPOST(t, core, nil, false, "/m/cpt.gguf") + if w.Code != http.StatusOK { + t.Fatalf("POST /models after a rollback = %d; want 200 with the failure rendered", w.Code) + } + body := w.Body.String() + if !strings.Contains(body, "rolled back to qwen3") { + t.Errorf("page does not say it rolled back:\n%s", body) + } +} + +func TestModels_RefusedPathIs403(t *testing.T) { + core := &fakeModelCore{swapErr: ipc.ErrForbidden} + w := modelsPOST(t, core, nil, false, "/etc/passwd") + if w.Code != http.StatusForbidden { + t.Fatalf("POST /models with a non-allowlisted path = %d; want 403", w.Code) + } +} + +func TestModels_UnconfiguredCoreRendersOff(t *testing.T) { + core := &fakeModelCore{statusErr: ipc.ErrUnknownMethod} + w := modelsGET(t, core) + if w.Code != http.StatusOK { + t.Fatalf("GET /models against a core without the swap = %d; want 200", w.Code) + } + if !strings.Contains(w.Body.String(), "swap not configured") { + t.Errorf("page does not say the capability is off:\n%s", w.Body.String()) + } +} + +func TestModels_CoreWithoutTheMethodsIs503(t *testing.T) { + // An in-process CoreAPI (no swap methods) must not 500 the page. + w := modelsGET(t, ipc.UnimplementedCoreAPI{}) + if w.Code != http.StatusServiceUnavailable { + t.Fatalf("GET /models on a core without the methods = %d; want 503", w.Code) + } +} + +type errBrokenModel struct{} + +func (errBrokenModel) Error() string { return "llm: server did not start" } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index c7c1872..67907af 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -393,3 +393,26 @@ func mustWriteFactParams(source string) []byte { } return b } + +// TestRequirement_SwapModel — loading a different resident model is an owner +// action at the same rung as mutating the tool allowlist: it decides how every +// utterance is routed and how every reply is worded. The read side is not. +func TestRequirement_SwapModel(t *testing.T) { + if got := Requirement(ipc.MethodSwapModel); got != AuthStepUp { + t.Errorf("SwapModel authority = %v; want AuthStepUp", got) + } + if got := Requirement(ipc.MethodModelStatus); got != AuthRead { + t.Errorf("ModelStatus authority = %v; want AuthRead", got) + } + // A surface that cannot carry a passkey gesture cannot swap the model, no + // matter what it is enrolled as — this is the "never through voice" property. + voice := Scope{Surface: SurfaceVoice, Module: "voice", SourceScope: []string{"*"}} + if err := Can(ipc.MethodSwapModel, voice, nil); !errors.Is(err, ErrForbidden) { + t.Errorf("voice swapping the model = %v; want ErrForbidden", err) + } + // And with no step-up session asserted, the gate refuses even a capable surface. + noSession := &Gate{Enrollment: NewFloorEnrollment()} + if err := noSession.Check(context.Background(), ipc.MethodSwapModel, nil); !errors.Is(err, ipc.ErrForbidden) { + t.Errorf("SwapModel with no asserted step-up = %v; want ErrForbidden", err) + } +} diff --git a/internal/auth/policy.go b/internal/auth/policy.go index 429edc3..ae6e9d0 100644 --- a/internal/auth/policy.go +++ b/internal/auth/policy.go @@ -53,6 +53,13 @@ func Requirement(m ipc.Method) Authority { // asserted — never a module or the voice/chat path. maven can propose // (MethodProposeTool, no step-up: she has no passkey) but never en/disable. return AuthStepUp + case ipc.MethodSwapModel: + // Swapping the resident model changes what routes every utterance and + // what words every reply. It is the owner's call, from a surface that can + // carry a passkey gesture — the same rung as mutating the tool allowlist, + // and for the same reason: nothing Maven says or does may reach it. + // MethodModelStatus is only the read side, so it stays at AuthRead. + return AuthStepUp case ipc.MethodWriteFact: return AuthWrite case ipc.MethodAssertStepUp: @@ -79,7 +86,10 @@ func Requirement(m ipc.Method) Authority { // produce: candidate tasks and nothing else. It cannot write a fact, set a // reminder, or touch the tool allowlist, so a compromised mail reader can // at worst put junk on a review page he clears in one click. - ipc.MethodIngestMail: + ipc.MethodIngestMail, + // The read side of the model swap: which model is resident, which ones are + // allowlisted. It loads nothing and changes nothing. + ipc.MethodModelStatus: return AuthRead } // Unknown method ⇒ AuthRead, but ipc.dispatch returns ErrUnknownMethod diff --git a/internal/config/config.go b/internal/config/config.go index c3fe4b6..579dfcc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -579,6 +579,21 @@ type PhraserConfig struct { // persona and invented units). Chat, query and reminder phrasing always go // through the model regardless. See phraser.Config.LLMNudges. LLMNudges bool `json:"llm_nudges,omitempty"` + + // SwapModels — the gguf files the running daemon is allowed to swap to + // without a restart (Vikunja #250). Empty (the default) means the swap + // capability does not exist: ipc.MethodSwapModel answers ErrUnknownMethod, + // exactly like an unconfigured weather or telegram block. + // + // It is an allowlist and not a directory on purpose. The request carries a + // path, and llama-server is started with it as `-m`; anything short of an + // exact match against a list a human wrote in this file would make "swap the + // model" mean "load a file of your choosing off my disk". ModelPath is + // always swappable back to whether or not it is listed. + // + // Paths must be absolute — the daemon's working directory is not the + // operator's, and a relative path here would resolve somewhere surprising. + SwapModels []string `json:"swap_models,omitempty"` } // EmbedderConfig — paths for the ONNX multilingual embedder. The daemon @@ -820,6 +835,14 @@ func (c *Config) validate() error { if c.Phraser.ModelPath == "" { return errors.New("phraser.model_path is required") } + // A relative entry in the swap allowlist would resolve against the + // daemon's working directory, so the path a human reads in this file + // would not be the path llama-server is handed. Fail at startup. + for _, m := range c.Phraser.SwapModels { + if !filepath.IsAbs(m) { + return fmt.Errorf("phraser.swap_models: %q must be an absolute path", m) + } + } } if c.Voice != nil && c.Voice.Enabled { if c.Voice.Bind == "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9f25ed1..4786e58 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -293,3 +293,35 @@ func TestPatternProposalNotifyDefaultsOff(t *testing.T) { t.Errorf("cooldown = %v, want 6h", c.PatternProposals.Cooldown) } } + +// TestSwapModelsAbsentMeansOff — the swap capability does not exist unless the +// operator lists the models he allows (Vikunja #250). +func TestSwapModelsAbsentMeansOff(t *testing.T) { + c, err := Load(writeConfig(t, `{"phraser": {"model_path": "/m/qwen.gguf"}}`)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Phraser.SwapModels) != 0 { + t.Errorf("swap_models = %v; want empty when unconfigured", c.Phraser.SwapModels) + } +} + +func TestSwapModelsParsedAndMustBeAbsolute(t *testing.T) { + c, err := Load(writeConfig(t, `{"phraser": { + "model_path": "/m/qwen.gguf", + "swap_models": ["/m/qwen.gguf", "/m/qwen-cpt.gguf"] + }}`)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(c.Phraser.SwapModels) != 2 { + t.Fatalf("swap_models = %v; want 2 entries", c.Phraser.SwapModels) + } + // A relative entry would resolve against the daemon's cwd, not the operator's. + if _, err := Load(writeConfig(t, `{"phraser": { + "model_path": "/m/qwen.gguf", + "swap_models": ["models/llm/qwen.gguf"] + }}`)); err == nil { + t.Error("Load accepted a relative swap_models entry; want a startup failure") + } +} diff --git a/internal/ipc/api.go b/internal/ipc/api.go index 28d7284..a9a0be7 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -176,6 +176,50 @@ type IngestMailResp struct { Skipped bool `json:"skipped,omitempty"` } +// SwapModelReq — load another resident model without restarting the daemon +// (Vikunja #250). ModelPath must be one of the paths in phraser.swap_models; +// anything else is ErrForbidden, and an unconfigured allowlist makes the whole +// method ErrUnknownMethod. +// +// NGpuLayers and NCtx are zero for "keep what is loaded now", which is the +// normal case — the same laptop iGPU, a different gguf. +// +// This is an owner action. It is AuthStepUp in the authority table, it is not on +// CoreAPI, and no act, intent or timer can reach it: swapping the model is not +// something Maven does to herself. +type SwapModelReq struct { + ModelPath string `json:"model_path"` + NGpuLayers int `json:"n_gpu_layers,omitempty"` + NCtx int `json:"n_ctx,omitempty"` +} + +// SwapModelResp — what the daemon ended up serving. Model is the identity the +// new llama-server reported for itself, not an echo of the request: if the file +// was not the model the operator thought it was, this is where it shows. +// +// RolledBack is true when the requested model failed to load or would not answer +// and the previous one was put back. In that case the call also returns an error +// — the swap did not happen — and Model names the model still serving. +type SwapModelResp struct { + Model string `json:"model"` + ModelPath string `json:"model_path"` + BaseURL string `json:"base_url"` + RolledBack bool `json:"rolled_back,omitempty"` + TookMs int64 `json:"took_ms"` +} + +// ModelStatusResp — which model is resident and which ones may be swapped in. +// Read-only; the authed page renders it. Swappable is the configured allowlist, +// so an empty list means the capability is off. +type ModelStatusResp struct { + Model string `json:"model"` + ModelPath string `json:"model_path"` + BaseURL string `json:"base_url"` + NGpuLayers int `json:"n_gpu_layers"` + NCtx int `json:"n_ctx"` + Swappable []string `json:"swappable,omitempty"` +} + type listTasksReq struct { Status string `json:"status"` // "" all | "live" | candidate|open|done|dropped } diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 3602bbb..898e1c5 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -459,6 +459,28 @@ func (c *Client) IngestMail(ctx context.Context, req IngestMailReq) (IngestMailR return r, nil } +// SwapModel asks core to load another resident model (Vikunja #250). +// ErrUnknownMethod means core has no phraser.swap_models allowlist configured; +// ErrForbidden means the path is not on it, or step-up was not asserted. A +// non-nil error with RolledBack set means nothing changed — the old model is +// still serving. +func (c *Client) SwapModel(ctx context.Context, req SwapModelReq) (SwapModelResp, error) { + var r SwapModelResp + if err := c.call(ctx, MethodSwapModel, req, &r); err != nil { + return SwapModelResp{}, err + } + return r, nil +} + +// ModelStatus reports the resident model and the swap allowlist. Read-only. +func (c *Client) ModelStatus(ctx context.Context) (ModelStatusResp, error) { + var r ModelStatusResp + if err := c.call(ctx, MethodModelStatus, nil, &r); err != nil { + return ModelStatusResp{}, err + } + return r, nil +} + func (c *Client) DismissProposedRoutine(ctx context.Context, id int64) error { return c.call(ctx, MethodDismissProposedRoutine, dismissProposedRoutineReq{ID: id}, nil) } diff --git a/internal/ipc/ipc_test.go b/internal/ipc/ipc_test.go index 86f6a9d..885d7bd 100644 --- a/internal/ipc/ipc_test.go +++ b/internal/ipc/ipc_test.go @@ -6,6 +6,7 @@ import ( "encoding/binary" "encoding/json" "errors" + "fmt" "io" "net" "os" @@ -598,3 +599,44 @@ func TestIngestMail_Hook(t *testing.T) { t.Errorf("req across the wire = %+v", got) } } + +// TestSwapModel_OffUnlessConfigured — no allowlist in the config means the +// daemon never sets the hook, and the method does not exist. That is what "off +// unless configured" looks like at the wire for the model swap (Vikunja #250). +func TestSwapModel_OffUnlessConfigured(t *testing.T) { + _, _, cli, _ := newServerWithStore(t) + if _, err := cli.SwapModel(context.Background(), SwapModelReq{ModelPath: "/m/x.gguf"}); !errors.Is(err, ErrUnknownMethod) { + t.Fatalf("SwapModel error = %v, want ErrUnknownMethod", err) + } + if _, err := cli.ModelStatus(context.Background()); !errors.Is(err, ErrUnknownMethod) { + t.Fatalf("ModelStatus error = %v, want ErrUnknownMethod", err) + } +} + +// TestSwapModel_Hook — the request crosses the boundary intact and the reported +// identity comes back. A refusal from the daemon's allowlist arrives as +// ErrForbidden, which is what a caller keys its error message off. +func TestSwapModel_Hook(t *testing.T) { + _, srv, cli, _ := newServerWithStore(t) + var got SwapModelReq + srv.SwapModelFn = func(_ context.Context, req SwapModelReq) (SwapModelResp, error) { + got = req + if req.ModelPath != "/m/allowed.gguf" { + return SwapModelResp{}, fmt.Errorf("%w: not allowlisted", ErrForbidden) + } + return SwapModelResp{Model: "allowed", ModelPath: req.ModelPath, BaseURL: "http://127.0.0.1:9", TookMs: 12}, nil + } + resp, err := cli.SwapModel(context.Background(), SwapModelReq{ModelPath: "/m/allowed.gguf", NCtx: 4096}) + if err != nil { + t.Fatalf("SwapModel: %v", err) + } + if resp.Model != "allowed" || resp.TookMs != 12 { + t.Errorf("resp = %+v", resp) + } + if got.NCtx != 4096 { + t.Errorf("req across the wire = %+v", got) + } + if _, err := cli.SwapModel(context.Background(), SwapModelReq{ModelPath: "/etc/shadow"}); !errors.Is(err, ErrForbidden) { + t.Fatalf("swap to a non-allowlisted path = %v; want ErrForbidden", err) + } +} diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 1137153..de072fe 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -432,6 +432,19 @@ type Server struct { // every CoreAPI implementation has to carry. IngestMailFn IngestMailFunc + // SwapModelFn / ModelStatusFn — the on-the-fly resident model swap (Vikunja + // #250) and its read side. Set by the daemon only when phraser.swap_models + // lists at least one model AND the phraser owns a llama-server; nil ⇒ both + // methods answer ErrUnknownMethod, which is what "off unless configured" + // looks like at the wire. + // + // They bypass CoreAPI for the same reason IngestMailFn does: this is not a + // store operation, it needs the daemon's llama-server, and no other CoreAPI + // implementation should have to carry it. MethodSwapModel is AuthStepUp in + // internal/auth — owner-triggered, never an act and never a timer. + SwapModelFn SwapModelFunc + ModelStatusFn ModelStatusFunc + // UnlockFn — unwraps the store encryption key from the wrapped blob using // the passkey credential public key, opens the encrypted store, and wires // the rest of the daemon (voice, loop, delivery). Set by the daemon when @@ -450,6 +463,12 @@ type WrapKeyFunc func(ctx context.Context, publicKey []byte) error // public key and completes daemon initialization. type UnlockFunc func(ctx context.Context, publicKey []byte) error +// SwapModelFunc — loads another resident model in place of the live one. +type SwapModelFunc func(ctx context.Context, req SwapModelReq) (SwapModelResp, error) + +// ModelStatusFunc — reports the resident model and the swap allowlist. +type ModelStatusFunc func(ctx context.Context) (ModelStatusResp, error) + // IngestMailFunc — core-side mail extraction. Returns what was captured. type IngestMailFunc func(ctx context.Context, req IngestMailReq) (IngestMailResp, error) @@ -620,8 +639,9 @@ func withoutParams[R any](fn func(ctx context.Context, api CoreAPI) (R, error)) // existed) as an argument — so SetAPI's runtime swap (the unlock transition) // is still honored on the very next request with no extra plumbing here. // -// MethodAssertStepUp, MethodStoreEncryptionKey, MethodUnlock and -// MethodIngestMail are NOT in this table: they bypass CoreAPI entirely +// MethodAssertStepUp, MethodStoreEncryptionKey, MethodUnlock, +// MethodIngestMail, MethodSwapModel and MethodModelStatus are NOT in this +// table: they bypass CoreAPI entirely // (s.StepUp / s.WrapKeyFn / s.UnlockFn / s.IngestMailFn), so dispatch // special-cases them before consulting the table. var methodTable = map[Method]handlerFunc{ @@ -874,6 +894,30 @@ func (s *Server) dispatch(ctx context.Context, req Request) (json.RawMessage, er return marshalResult(resp), nil } return nil, fmt.Errorf("%w: %s", ErrUnknownMethod, req.Method) + + case MethodSwapModel: + if s.SwapModelFn != nil { + var p SwapModelReq + if err := unmarshalParams(req.Params, &p); err != nil { + return nil, err + } + resp, err := s.SwapModelFn(ctx, p) + if err != nil { + return nil, err + } + return marshalResult(resp), nil + } + return nil, fmt.Errorf("%w: %s", ErrUnknownMethod, req.Method) + + case MethodModelStatus: + if s.ModelStatusFn != nil { + resp, err := s.ModelStatusFn(ctx) + if err != nil { + return nil, err + } + return marshalResult(resp), nil + } + return nil, fmt.Errorf("%w: %s", ErrUnknownMethod, req.Method) } h, ok := methodTable[req.Method] diff --git a/internal/ipc/wire.go b/internal/ipc/wire.go index 3d1ea08..151eebf 100644 --- a/internal/ipc/wire.go +++ b/internal/ipc/wire.go @@ -51,6 +51,8 @@ const ( MethodListTasks Method = "list_tasks" MethodSetTaskStatus Method = "set_task_status" MethodIngestMail Method = "ingest_mail" + MethodSwapModel Method = "swap_model" + MethodModelStatus Method = "model_status" ) // Request — one frame from module to core. Params is the JSON-encoded argument diff --git a/internal/llm/client.go b/internal/llm/client.go index 7760296..a43748f 100644 --- a/internal/llm/client.go +++ b/internal/llm/client.go @@ -10,10 +10,17 @@ import ( "encoding/json" "fmt" "net/http" + "sync" "time" ) type Client struct { + // mu guards base only. The base URL changes when the daemon swaps the + // resident model (Vikunja #250): llama-server is relaunched on a fresh + // port, and every holder of this client — the LLM router, the replier, the + // mail extractor — must follow without being rebuilt. One mutexed field is + // the whole mechanism; a swap re-points the client, it does not replace it. + mu sync.RWMutex base string http *http.Client } @@ -22,6 +29,25 @@ func New(baseURL string, timeout time.Duration) *Client { return &Client{base: baseURL, http: &http.Client{Timeout: timeout}} } +// SetBaseURL re-points the client at another llama-server. Safe to call while +// requests are in flight: a request that already read the old base finishes +// against the old base (or fails, and every caller of Complete has a fallback), +// and the next one uses the new base. It is deliberately NOT a queue-and-retry — +// the phraser quiesces around a swap, so the window is small and a lost turn +// degrades to the classifier rather than hanging. +func (c *Client) SetBaseURL(base string) { + c.mu.Lock() + c.base = base + c.mu.Unlock() +} + +// BaseURL is the server this client currently talks to. +func (c *Client) BaseURL() string { + c.mu.RLock() + defer c.mu.RUnlock() + return c.base +} + type Req struct { System string User string @@ -63,7 +89,7 @@ func (c *Client) Complete(ctx context.Context, r Req) (string, error) { RepeatPenalty: r.RepeatPenalty, Stop: r.Stop, }) - req, err := http.NewRequestWithContext(ctx, "POST", c.base+"/v1/chat/completions", bytes.NewReader(b)) + req, err := http.NewRequestWithContext(ctx, "POST", c.BaseURL()+"/v1/chat/completions", bytes.NewReader(b)) if err != nil { return "", err } diff --git a/internal/llm/client_test.go b/internal/llm/client_test.go index 68f174d..9526c72 100644 --- a/internal/llm/client_test.go +++ b/internal/llm/client_test.go @@ -57,3 +57,37 @@ func TestComplete(t *testing.T) { t.Errorf("got %q, want %q", got, "ok") } } + +// TestSetBaseURL — a model swap re-points every holder of the client rather than +// rebuilding the router, the replier and the extractors (Vikunja #250). +func TestSetBaseURL(t *testing.T) { + var hit string + srvA := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hit = "A" + w.Write([]byte(`{"choices":[{"message":{"content":"a"}}]}`)) + })) + defer srvA.Close() + srvB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hit = "B" + w.Write([]byte(`{"choices":[{"message":{"content":"b"}}]}`)) + })) + defer srvB.Close() + + c := New(srvA.URL, 5*time.Second) + if _, err := c.Complete(context.Background(), Req{User: "x"}); err != nil { + t.Fatalf("Complete against A: %v", err) + } + if hit != "A" { + t.Fatalf("first request went to %q; want A", hit) + } + c.SetBaseURL(srvB.URL) + if got := c.BaseURL(); got != srvB.URL { + t.Errorf("BaseURL = %q; want %q", got, srvB.URL) + } + if _, err := c.Complete(context.Background(), Req{User: "x"}); err != nil { + t.Fatalf("Complete against B: %v", err) + } + if hit != "B" { + t.Errorf("request after the swap went to %q; want B", hit) + } +} diff --git a/internal/phraser/llmphraser.go b/internal/phraser/llmphraser.go index 558bec2..4006009 100644 --- a/internal/phraser/llmphraser.go +++ b/internal/phraser/llmphraser.go @@ -27,14 +27,46 @@ var listenRE = regexp.MustCompile(`listening on (https?://\S+)`) type LLMPhraser struct { cfg Config client *http.Client - port string - cmd *exec.Cmd - cancel context.CancelFunc - wg sync.WaitGroup // tmpl — the hand-written Russian nudges. Default path for nudges; see // Config.LLMNudges. nil only if the template file failed to load. tmpl *NudgeTemplates + + // spawnCtx — the parent of every llama-server this phraser starts, i.e. the + // daemon's own context. Deliberately NOT the per-request context of the call + // that asked for a model swap: that one is cancelled the moment the request + // returns, which would kill the model it had just loaded. + spawnCtx context.Context + cancel context.CancelFunc + + // launch / probe — the two side effects of a swap, injectable so the swap + // logic is testable without a real llama-server and a real model file. + // launch is nil when this phraser does not own its server (NewLLMPhraserAt), + // which is also what makes Swap refuse there. + launch func(ctx context.Context, cfg Config) (backend, error) + probe func(ctx context.Context, base string) (string, error) + + // swapMu — single-flight around Swap. Held for the whole swap, including the + // model load, so two concurrent swap requests can never both be loading. + swapMu sync.Mutex + + // mu guards everything below: the live backend, the swap gate and the + // in-flight request count. See acquire/quiesce in swap.go. + mu sync.Mutex + be backend + live liveModel + swapping bool + inflight int + observers []func(baseURL string) +} + +// liveModel — what is actually loaded right now. Distinct from Config, which +// stays immutable after construction: a swap changes these three fields and +// nothing else, so no reader of cfg (prompts, grammar, timeouts) races a swap. +type liveModel struct { + ModelPath string + NGpuLayers int + NCtx int } type Config struct { @@ -85,15 +117,21 @@ func DefaultConfig(modelPath string) Config { func NewLLMPhraser(ctx context.Context, cfg Config) (*LLMPhraser, error) { ctx, cancel := context.WithCancel(ctx) p := &LLMPhraser{ - cfg: cfg, - client: &http.Client{Timeout: cfg.Timeout}, - cancel: cancel, - tmpl: loadNudgeTemplates(), + cfg: cfg, + client: &http.Client{Timeout: cfg.Timeout}, + tmpl: loadNudgeTemplates(), + spawnCtx: ctx, + cancel: cancel, + launch: spawnLlamaServer, + probe: defaultProbe, + live: liveModel{ModelPath: cfg.ModelPath, NGpuLayers: cfg.NGpuLayers, NCtx: cfg.NCtx}, } - if err := p.start(ctx); err != nil { + be, err := p.launch(ctx, cfg) + if err != nil { cancel() return nil, err } + p.be = be return p, nil } @@ -106,11 +144,17 @@ func NewLLMPhraser(ctx context.Context, cfg Config) (*LLMPhraser, error) { // still uses NewLLMPhraser and still owns its own child process. func NewLLMPhraserAt(baseURL string, cfg Config) *LLMPhraser { return &LLMPhraser{ - cfg: cfg, - client: &http.Client{Timeout: cfg.Timeout}, - port: strings.TrimSuffix(baseURL, "/"), - cancel: func() {}, - tmpl: loadNudgeTemplates(), + cfg: cfg, + client: &http.Client{Timeout: cfg.Timeout}, + tmpl: loadNudgeTemplates(), + spawnCtx: context.Background(), + cancel: func() {}, + probe: defaultProbe, + // launch stays nil: we did not start this server, so we must not stop it. + // Swap therefore refuses here (ErrSwapNotOwned) instead of killing a + // server another process depends on. + be: borrowedBackend(strings.TrimSuffix(baseURL, "/")), + live: liveModel{ModelPath: cfg.ModelPath, NGpuLayers: cfg.NGpuLayers, NCtx: cfg.NCtx}, } } @@ -126,16 +170,66 @@ func loadNudgeTemplates() *NudgeTemplates { return nt } -func (p *LLMPhraser) start(ctx context.Context) error { +// backend — one llama-server this phraser talks to. Two implementations: a +// llamaProc we spawned and must reap, and a borrowedBackend someone else owns. +type backend interface { + BaseURL() string + Close() error +} + +// borrowedBackend — a server started and owned by someone else (the phrasing +// scorer's shared llama-server). Closing it is a no-op by construction. +type borrowedBackend string + +func (b borrowedBackend) BaseURL() string { return string(b) } +func (b borrowedBackend) Close() error { return nil } + +// llamaProc — a llama-server child process plus the goroutine reading its +// stderr. Close kills and reaps it; see the Pdeathsig note in spawnLlamaServer. +type llamaProc struct { + base string + cmd *exec.Cmd + cancel context.CancelFunc + wg sync.WaitGroup +} + +func (l *llamaProc) BaseURL() string { return l.base } + +func (l *llamaProc) Close() error { + l.cancel() + if l.cmd != nil && l.cmd.Process != nil { + _ = l.cmd.Process.Kill() + _ = l.cmd.Wait() // reap the process — without Wait, the child becomes a zombie + } + l.wg.Wait() + return nil +} + +// spawnLlamaServer starts one llama-server for cfg and waits until it says which +// address it is listening on. ctx owns the process lifetime, so it must be the +// daemon's context, not a request's. +func spawnLlamaServer(ctx context.Context, cfg Config) (backend, error) { + ctx, cancel := context.WithCancel(ctx) + p, err := startLlamaProc(ctx, cfg) + if err != nil { + cancel() + return nil, err + } + p.cancel = cancel + return p, nil +} + +func startLlamaProc(ctx context.Context, cfg Config) (*llamaProc, error) { + p := &llamaProc{} args := []string{ - "-m", p.cfg.ModelPath, + "-m", cfg.ModelPath, "--host", "127.0.0.1", - "--port", extractPort(p.cfg.Listen), - "-c", fmt.Sprintf("%d", p.cfg.NCtx), - "-ngl", fmt.Sprintf("%d", p.cfg.NGpuLayers), + "--port", extractPort(cfg.Listen), + "-c", fmt.Sprintf("%d", cfg.NCtx), + "-ngl", fmt.Sprintf("%d", cfg.NGpuLayers), "--no-webui", } - cmd := exec.CommandContext(ctx, p.cfg.BinPath, args...) + cmd := exec.CommandContext(ctx, cfg.BinPath, args...) // Pdeathsig: the kernel SIGKILLs llama-server the moment mavend dies — by // ANY means, including SIGKILL/OOM/panic where our Close() never runs. Without // it a hard-killed mavend orphans its llama-server (reparented to init, keeps @@ -148,12 +242,12 @@ func (p *LLMPhraser) start(ctx context.Context) error { stderr, err := cmd.StderrPipe() if err != nil { - return fmt.Errorf("llm: stderr pipe: %w", err) + return nil, fmt.Errorf("llm: stderr pipe: %w", err) } if err := cmd.Start(); err != nil { stderr.Close() - return fmt.Errorf("llm: start: %w", err) + return nil, fmt.Errorf("llm: start: %w", err) } portCh := make(chan string, 1) @@ -186,32 +280,44 @@ func (p *LLMPhraser) start(ctx context.Context) error { select { case addr := <-portCh: - p.port = addr - return nil + p.base = addr + return p, nil case err := <-errCh: _ = cmd.Process.Kill() _ = cmd.Wait() - return fmt.Errorf("llm: server output: %w", err) + return nil, fmt.Errorf("llm: server output: %w", err) case <-ctx.Done(): _ = cmd.Process.Kill() _ = cmd.Wait() - return ctx.Err() + return nil, ctx.Err() case <-time.After(60 * time.Second): _ = cmd.Process.Kill() _ = cmd.Wait() - return fmt.Errorf("llm: server did not start within 60s") + return nil, fmt.Errorf("llm: server did not start within 60s") } } -func (p *LLMPhraser) BaseURL() string { return p.port } +// BaseURL is the llama-server this phraser talks to right now. It changes when +// the model is swapped, so callers that cache it must register an observer +// (OnSwap) rather than keeping the string forever. +func (p *LLMPhraser) BaseURL() string { + p.mu.Lock() + defer p.mu.Unlock() + if p.be == nil { + return "" + } + return p.be.BaseURL() +} func (p *LLMPhraser) Close() error { p.cancel() - if p.cmd != nil && p.cmd.Process != nil { - _ = p.cmd.Process.Kill() - _ = p.cmd.Wait() // reap the process — without Wait, the child becomes a zombie + p.mu.Lock() + be := p.be + p.be = nil + p.mu.Unlock() + if be != nil { + return be.Close() } - p.wg.Wait() return nil } @@ -350,6 +456,11 @@ func chatSystemPrompt(block func() string) string { // the LLM completion endpoint. Like chatWithSystem but for an arbitrary message // slice — the caller owns the system prompt placement. func (p *LLMPhraser) chatWithMessages(ctx context.Context, msgs []chatMsg, maxTokens int) (string, error) { + base, release, err := p.acquire() + if err != nil { + return "", err + } + defer release() req := chatReq{ Messages: msgs, Temperature: 0.7, @@ -360,7 +471,7 @@ func (p *LLMPhraser) chatWithMessages(ctx context.Context, msgs []chatMsg, maxTo if err != nil { return "", fmt.Errorf("llm: marshal: %w", err) } - httpReq, err := http.NewRequestWithContext(ctx, "POST", p.port+"/v1/chat/completions", bytes.NewReader(body)) + httpReq, err := http.NewRequestWithContext(ctx, "POST", base+"/v1/chat/completions", bytes.NewReader(body)) if err != nil { return "", fmt.Errorf("llm: request: %w", err) } @@ -493,6 +604,11 @@ func (p *LLMPhraser) chat(ctx context.Context, userPrompt string) (string, error } func (p *LLMPhraser) chatWithSystem(ctx context.Context, system, user string, maxTokens int) (string, error) { + base, release, err := p.acquire() + if err != nil { + return "", err + } + defer release() req := chatReq{ Messages: []chatMsg{ {Role: "system", Content: system}, @@ -507,7 +623,7 @@ func (p *LLMPhraser) chatWithSystem(ctx context.Context, system, user string, ma return "", fmt.Errorf("llm: marshal: %w", err) } - httpReq, err := http.NewRequestWithContext(ctx, "POST", p.port+"/v1/chat/completions", bytes.NewReader(body)) + httpReq, err := http.NewRequestWithContext(ctx, "POST", base+"/v1/chat/completions", bytes.NewReader(body)) if err != nil { return "", fmt.Errorf("llm: request: %w", err) } diff --git a/internal/phraser/swap.go b/internal/phraser/swap.go new file mode 100644 index 0000000..22d9aeb --- /dev/null +++ b/internal/phraser/swap.go @@ -0,0 +1,330 @@ +package phraser + +import ( + "context" + "errors" + "fmt" + "log" + "time" + + "github.com/kami/maven/internal/llm" +) + +// Swapping the resident model without restarting the daemon (Vikunja #250). +// +// Three properties this file exists to hold, in order of importance: +// +// 1. NEVER two models resident at once. The deploy target is a laptop iGPU +// with the whole 1.7B offloaded to it (`n_gpu_layers: 99`); loading a second +// model beside the first is how you OOM the box, and a blue/green swap that +// "keeps the old one warm until the new one answers" does exactly that. So +// the old server is killed FIRST and the new one loaded after. The cost of +// that ordering is a window with no model at all, which is why: +// +// 2. A swap is atomic from a turn's point of view. An in-flight turn finishes +// on the old model — Swap waits for the last one to return before killing +// anything. A turn that arrives during the swap is REFUSED immediately with +// ErrSwapping rather than blocked: every phrasing path already has a +// fallback (templates, "вот что я нашла", the classifier for routing), so a +// fast refusal degrades one turn instead of hanging it for the length of a +// model load. No turn ever gets half of one model and half of another. +// +// 3. A failed load rolls back to the model that was working. The new server is +// probed (it must say which model it loaded) before it is published; if the +// launch or the probe fails, the previous config is relaunched and the +// phraser goes back to serving. Only if the rollback ALSO fails is the +// phraser left without a backend, and then it says so loudly and every turn +// degrades rather than breaks. +// +// Not here, deliberately: nothing calls Swap on a timer, and no act or intent can +// reach it. It is an IPC method behind the step-up gate, i.e. owner-triggered. + +var ( + // ErrSwapping — a turn arrived while the model was being swapped. Callers + // treat it like any other LLM error and use their fallback. + ErrSwapping = errors.New("phraser: model swap in progress") + + // ErrSwapNotOwned — this phraser did not start its llama-server, so it must + // not stop one (NewLLMPhraserAt: the eval harness shares a server). + ErrSwapNotOwned = errors.New("phraser: llama-server is not ours to swap") + + // ErrNoBackend — no model is loaded at all. Only reachable after a failed + // swap whose rollback also failed. + ErrNoBackend = errors.New("phraser: no llama-server loaded") + + // ErrSwapBusy — a turn was still running when the drain deadline expired, so + // the swap was abandoned. Nothing was killed; ask again. + ErrSwapBusy = errors.New("phraser: turns still in flight, swap abandoned") +) + +// SwapSpec — what to load. Zero NGpuLayers/NCtx keep whatever is live, so the +// common case ("same settings, different gguf") is one field. +type SwapSpec struct { + ModelPath string + NGpuLayers int + NCtx int +} + +// SwapResult — what happened. Model is the identity the NEW server reported, so +// it is evidence rather than an echo of the request: if the file at ModelPath is +// not what the operator thought it was, this is where that shows up. +type SwapResult struct { + Model string + BaseURL string + ModelPath string + RolledBack bool + Took time.Duration +} + +// drainTimeout — how long Swap waits for in-flight turns before giving up. A +// turn is at most Config.Timeout (30s in deploy) plus the model's own latency; +// 90s covers a slow Thinking generation without wedging the caller forever. +const drainTimeout = 90 * time.Second + +// probeTimeout — how long the new server gets to answer "which model do you +// have". The load itself is bounded by spawnLlamaServer's own 60s wait. +const probeTimeout = 30 * time.Second + +// defaultProbe asks the server which model it has loaded. This is the health +// check: a server that answers /v1/models has finished loading weights and is +// serving, and its answer is the identity we report back. +func defaultProbe(ctx context.Context, base string) (string, error) { + return llm.ModelID(ctx, base) +} + +// OnSwap registers a callback fired with the new base URL every time the live +// backend changes, including after a rollback. Holders of an *llm.Client (the +// LLM router, the replier, the mail extractor) register SetBaseURL here so a +// swap re-points them without rebuilding the router or the handler. +// +// Callbacks run with no lock held, in registration order. +func (p *LLMPhraser) OnSwap(fn func(baseURL string)) { + if fn == nil { + return + } + p.mu.Lock() + p.observers = append(p.observers, fn) + p.mu.Unlock() +} + +// LiveModel is the model file currently loaded (and its load settings). Empty +// ModelPath means no model is loaded. +func (p *LLMPhraser) LiveModel() (path string, nGpuLayers, nCtx int) { + p.mu.Lock() + defer p.mu.Unlock() + return p.live.ModelPath, p.live.NGpuLayers, p.live.NCtx +} + +// acquire reserves a slot for one request and returns the base URL to use. +// Every request path must call it and must call the returned release exactly +// once — that count is what Swap drains. +func (p *LLMPhraser) acquire() (string, func(), error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.swapping { + return "", nil, ErrSwapping + } + if p.be == nil { + return "", nil, ErrNoBackend + } + p.inflight++ + base := p.be.BaseURL() + var once bool + return base, func() { + p.mu.Lock() + if !once { + once = true + p.inflight-- + } + p.mu.Unlock() + }, nil +} + +// Swap loads another model in place of the live one. See the file comment for +// the properties it guarantees. Returns the new model's reported identity, or +// an error plus RolledBack=true when the old model was put back. +// +// ctx bounds the drain and the probe. It does NOT own the new server's lifetime +// — that is the daemon's context, captured at construction — so a swap survives +// the request that asked for it. +func (p *LLMPhraser) Swap(ctx context.Context, spec SwapSpec) (SwapResult, error) { + if spec.ModelPath == "" { + return SwapResult{}, fmt.Errorf("phraser: swap needs a model path") + } + p.swapMu.Lock() + defer p.swapMu.Unlock() + + if p.launch == nil { + return SwapResult{}, ErrSwapNotOwned + } + + started := time.Now() + oldLive := p.liveSnapshot() + newLive := liveModel{ + ModelPath: spec.ModelPath, + NGpuLayers: pickInt(spec.NGpuLayers, oldLive.NGpuLayers), + NCtx: pickInt(spec.NCtx, oldLive.NCtx), + } + if newLive == oldLive && p.BaseURL() != "" { + // Already serving exactly this. Report the live identity rather than + // pointlessly unloading and reloading the same weights. + base := p.BaseURL() + id, err := p.probeWith(ctx, base) + if err != nil { + return SwapResult{}, err + } + return SwapResult{Model: id, BaseURL: base, ModelPath: oldLive.ModelPath, Took: time.Since(started)}, nil + } + + if err := p.quiesce(ctx); err != nil { + return SwapResult{}, err + } + defer p.resume() + + // Property 1: the old model leaves the GPU before the new one arrives. + p.mu.Lock() + old := p.be + p.be = nil + p.mu.Unlock() + if old != nil { + _ = old.Close() + } + + be, err := p.loadAndProbe(ctx, newLive) + if err != nil { + log.Printf("phraser: swap to %s FAILED (%v) — rolling back to %s", newLive.ModelPath, err, oldLive.ModelPath) + rb, rbErr := p.loadAndProbe(ctx, oldLive) + if rbErr != nil { + log.Printf("phraser: ROLLBACK to %s ALSO FAILED (%v) — no model is loaded, every phrasing path is on its fallback and routing is on the classifier until the daemon is restarted", oldLive.ModelPath, rbErr) + return SwapResult{RolledBack: true, Took: time.Since(started)}, + fmt.Errorf("phraser: swap failed (%w) and rollback failed too: %v", err, rbErr) + } + p.publish(rb, oldLive) + return SwapResult{ + Model: rb.id, BaseURL: rb.be.BaseURL(), ModelPath: oldLive.ModelPath, + RolledBack: true, Took: time.Since(started), + }, + fmt.Errorf("phraser: swap to %s failed, rolled back to %s: %w", newLive.ModelPath, oldLive.ModelPath, err) + } + p.publish(be, newLive) + log.Printf("phraser: model swapped to %s (%s) at %s in %s", newLive.ModelPath, be.id, be.be.BaseURL(), time.Since(started).Round(time.Millisecond)) + return SwapResult{ + Model: be.id, BaseURL: be.be.BaseURL(), ModelPath: newLive.ModelPath, + Took: time.Since(started), + }, nil +} + +// loaded — a started server plus the identity it reported. +type loaded struct { + be backend + id string +} + +// loadAndProbe starts a server for lm and verifies it answers. A server that +// starts but will not say what it loaded is treated as a failed load and is +// killed here — publishing it would hand every turn to a backend we could not +// confirm. +func (p *LLMPhraser) loadAndProbe(ctx context.Context, lm liveModel) (loaded, error) { + cfg := p.cfg + cfg.ModelPath = lm.ModelPath + cfg.NGpuLayers = lm.NGpuLayers + cfg.NCtx = lm.NCtx + // p.spawnCtx, not ctx: the process must outlive the request asking for it. + be, err := p.launch(p.spawnCtx, cfg) + if err != nil { + return loaded{}, err + } + id, err := p.probeWith(ctx, be.BaseURL()) + if err != nil { + _ = be.Close() + return loaded{}, fmt.Errorf("phraser: %s started but would not answer: %w", lm.ModelPath, err) + } + return loaded{be: be, id: id}, nil +} + +func (p *LLMPhraser) probeWith(ctx context.Context, base string) (string, error) { + probe := p.probe + if probe == nil { + probe = defaultProbe + } + pctx, cancel := context.WithTimeout(ctx, probeTimeout) + defer cancel() + return probe(pctx, base) +} + +// quiesce closes the door on new turns and waits for the ones already running. +// Polling rather than a sync.Cond: the wait happens once per swap, a 25ms poll +// is invisible next to a model load, and a poll cannot deadlock on a release +// path that panicked. +func (p *LLMPhraser) quiesce(ctx context.Context) error { + p.mu.Lock() + if p.swapping { + p.mu.Unlock() + return ErrSwapping + } + p.swapping = true + inflight := p.inflight + p.mu.Unlock() + if inflight == 0 { + return nil + } + + deadline := time.Now().Add(drainTimeout) + for { + select { + case <-ctx.Done(): + p.resume() + return ctx.Err() + case <-time.After(25 * time.Millisecond): + } + p.mu.Lock() + inflight = p.inflight + p.mu.Unlock() + if inflight == 0 { + return nil + } + if time.Now().After(deadline) { + // Nothing has been killed yet, so abandoning is free: reopen the door + // and let the operator try again rather than cutting a live turn off + // mid-generation. + p.resume() + return fmt.Errorf("%w (%d still running after %s)", ErrSwapBusy, inflight, drainTimeout) + } + } +} + +func (p *LLMPhraser) resume() { + p.mu.Lock() + p.swapping = false + p.mu.Unlock() +} + +// publish makes l the live backend and tells everyone holding a base URL. +func (p *LLMPhraser) publish(l loaded, lm liveModel) { + p.mu.Lock() + p.be = l.be + p.live = lm + obs := make([]func(string), len(p.observers)) + copy(obs, p.observers) + p.mu.Unlock() + base := l.be.BaseURL() + for _, fn := range obs { + fn(base) + } +} + +func (p *LLMPhraser) liveSnapshot() liveModel { + p.mu.Lock() + defer p.mu.Unlock() + return p.live +} + +// pickInt returns v when the caller set it, and fallback otherwise. 0 is the +// "unset" value: -1 already means "offload every layer" and deploy uses 99, so +// nothing legitimate asks for exactly zero GPU layers through this path. +func pickInt(v, fallback int) int { + if v == 0 { + return fallback + } + return v +} diff --git a/internal/phraser/swap_test.go b/internal/phraser/swap_test.go new file mode 100644 index 0000000..aea6f39 --- /dev/null +++ b/internal/phraser/swap_test.go @@ -0,0 +1,311 @@ +package phraser + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + "time" +) + +// fakeModel — a stand-in llama-server. It answers /v1/models with its own name +// and /v1/chat/completions with a phrasing-contract reply that names itself, so +// a test can tell WHICH model answered a turn — the property the swap is about. +type fakeModel struct { + srv *httptest.Server + name string + closed atomic.Bool +} + +func newFakeModel(t *testing.T, name string) *fakeModel { + t.Helper() + f := &fakeModel{name: name} + mux := http.NewServeMux() + mux.HandleFunc("/v1/models", func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{"data":[{"id":"/models/` + name + `.gguf"}]}`)) + }) + mux.HandleFunc("/v1/chat/completions", func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{"choices":[{"message":{"content":"{\"response\":\"` + name + `\",\"mood\":\"neutral\"}"}}]}`)) + }) + f.srv = httptest.NewServer(mux) + t.Cleanup(f.srv.Close) + return f +} + +func (f *fakeModel) BaseURL() string { return f.srv.URL } +func (f *fakeModel) Close() error { f.closed.Store(true); return nil } + +// fakeFleet is the injected launcher: it hands out a prepared fakeModel per +// model path, and refuses paths the test did not prepare (that is what a bad +// gguf looks like from here). It also asserts the invariant that matters on a +// laptop iGPU: never two servers alive at the same time. +type fakeFleet struct { + mu sync.Mutex + models map[string]string // model path → fake name + live int + maxLive int + launch int +} + +func (fl *fakeFleet) launcher(t *testing.T) func(context.Context, Config) (backend, error) { + return func(ctx context.Context, cfg Config) (backend, error) { + fl.mu.Lock() + name, ok := fl.models[cfg.ModelPath] + fl.launch++ + if !ok { + fl.mu.Unlock() + return nil, errors.New("no such model file: " + cfg.ModelPath) + } + fl.live++ + if fl.live > fl.maxLive { + fl.maxLive = fl.live + } + fl.mu.Unlock() + f := newFakeModel(t, name) + return &fleetBackend{fleet: fl, model: f}, nil + } +} + +type fleetBackend struct { + fleet *fakeFleet + model *fakeModel + once sync.Once +} + +func (b *fleetBackend) BaseURL() string { return b.model.BaseURL() } +func (b *fleetBackend) Close() error { + b.once.Do(func() { + b.fleet.mu.Lock() + b.fleet.live-- + b.fleet.mu.Unlock() + }) + return b.model.Close() +} + +// newSwapPhraser builds an LLMPhraser with an injected launcher, so the swap +// path is exercised without a gguf or a GPU. +func newSwapPhraser(t *testing.T, fl *fakeFleet, modelPath string) *LLMPhraser { + t.Helper() + cfg := DefaultConfig(modelPath) + cfg.Timeout = 5 * time.Second + p := &LLMPhraser{ + cfg: cfg, + client: &http.Client{Timeout: cfg.Timeout}, + spawnCtx: context.Background(), + cancel: func() {}, + launch: fl.launcher(t), + probe: defaultProbe, + live: liveModel{ModelPath: modelPath, NGpuLayers: cfg.NGpuLayers, NCtx: cfg.NCtx}, + } + be, err := p.launch(p.spawnCtx, cfg) + if err != nil { + t.Fatalf("initial launch: %v", err) + } + p.be = be + t.Cleanup(func() { p.Close() }) + return p +} + +func TestSwap_LoadsNewModelAndRepointsHolders(t *testing.T) { + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old", "/m/new.gguf": "new"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + + // A holder of the base URL (the LLM router's client, in the daemon). + var seen []string + p.OnSwap(func(base string) { seen = append(seen, base) }) + + before, err := p.PhraseChat(context.Background(), "привет", nil) + if err != nil || before != "old" { + t.Fatalf("before swap: %q, %v; want the old model to answer", before, err) + } + + res, err := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/new.gguf"}) + if err != nil { + t.Fatalf("Swap: %v", err) + } + if res.Model != "new" { + t.Errorf("res.Model = %q; want the identity the NEW server reported (%q)", res.Model, "new") + } + if res.RolledBack { + t.Errorf("res.RolledBack = true on a successful swap") + } + after, err := p.PhraseChat(context.Background(), "привет", nil) + if err != nil || after != "new" { + t.Fatalf("after swap: %q, %v; want the new model to answer", after, err) + } + if path, _, _ := p.LiveModel(); path != "/m/new.gguf" { + t.Errorf("LiveModel = %q; want /m/new.gguf", path) + } + if len(seen) != 1 || seen[0] != p.BaseURL() { + t.Errorf("observers saw %v; want exactly one call with the new base %q", seen, p.BaseURL()) + } + if fl.maxLive > 1 { + t.Errorf("%d servers were alive at once; the iGPU only fits one model", fl.maxLive) + } +} + +func TestSwap_FailedLoadRollsBackToTheWorkingModel(t *testing.T) { + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + + res, err := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/broken.gguf"}) + if err == nil { + t.Fatal("Swap to a model that will not load returned nil error") + } + if !res.RolledBack { + t.Errorf("res.RolledBack = false; a failed swap must say it rolled back") + } + if res.Model != "old" { + t.Errorf("res.Model = %q; want the old model back", res.Model) + } + // The point of the rollback: turns keep working. + got, err := p.PhraseChat(context.Background(), "привет", nil) + if err != nil || got != "old" { + t.Fatalf("after rollback: %q, %v; want the old model serving again", got, err) + } + if path, _, _ := p.LiveModel(); path != "/m/old.gguf" { + t.Errorf("LiveModel = %q; want the old model", path) + } + if fl.maxLive > 1 { + t.Errorf("%d servers alive at once during a rollback", fl.maxLive) + } +} + +func TestSwap_ProbeFailureIsTreatedAsAFailedLoad(t *testing.T) { + // A server that starts but will not say what it loaded must never be + // published — we would be serving turns from a backend we cannot confirm. + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old", "/m/mute.gguf": "mute"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + // Fail the probe once — for the newly launched server — and let the + // rollback's probe through. + calls := 0 + p.probe = func(ctx context.Context, base string) (string, error) { + calls++ + if calls == 1 { + return "", errors.New("no answer from the new server") + } + return defaultProbe(ctx, base) + } + + _, err := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/mute.gguf"}) + if err == nil { + t.Fatal("Swap published a server that failed its probe") + } + if path, _, _ := p.LiveModel(); path != "/m/old.gguf" { + t.Errorf("LiveModel = %q; want the old model after a failed probe", path) + } +} + +func TestSwap_RollbackFailureLeavesNoBackendAndDegrades(t *testing.T) { + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + // Make the rollback fail too: the old file "disappears" mid-swap. + fl.mu.Lock() + delete(fl.models, "/m/old.gguf") + fl.mu.Unlock() + + _, err := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/broken.gguf"}) + if err == nil { + t.Fatal("Swap returned nil when both the load and the rollback failed") + } + // Nothing is loaded, and the request path says so rather than panicking. + if _, _, aerr := p.acquire(); !errors.Is(aerr, ErrNoBackend) { + t.Errorf("acquire error = %v; want ErrNoBackend", aerr) + } + // Phrasing degrades to its fallback instead of failing the turn. + got, err := p.PhraseChat(context.Background(), "привет", nil) + if err != nil { + t.Fatalf("PhraseChat after a total failure returned an error: %v", err) + } + if got == "" { + t.Error("PhraseChat returned empty; the fallback must still say something") + } +} + +func TestSwap_WaitsForInFlightTurnAndRefusesNewOnes(t *testing.T) { + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old", "/m/new.gguf": "new"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + + // Hold one turn open by taking a slot directly — the same slot every + // request path takes. + base, release, err := p.acquire() + if err != nil { + t.Fatalf("acquire: %v", err) + } + if base == "" { + t.Fatal("acquire returned an empty base URL") + } + + swapped := make(chan error, 1) + go func() { _, e := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/new.gguf"}); swapped <- e }() + + // While the swap waits to drain, a NEW turn is refused immediately rather + // than blocked for the length of a model load. + deadline := time.Now().Add(2 * time.Second) + for { + _, rel, aerr := p.acquire() + if rel != nil { + rel() + } + if errors.Is(aerr, ErrSwapping) { + break + } + if time.Now().After(deadline) { + t.Fatalf("new turns were never refused during a swap (last error: %v)", aerr) + } + time.Sleep(10 * time.Millisecond) + } + + // The swap cannot have completed while our turn was still in flight. + select { + case e := <-swapped: + t.Fatalf("Swap finished before the in-flight turn released: %v", e) + case <-time.After(50 * time.Millisecond): + } + + release() + if e := <-swapped; e != nil { + t.Fatalf("Swap after drain: %v", e) + } + got, err := p.PhraseChat(context.Background(), "привет", nil) + if err != nil || got != "new" { + t.Fatalf("after swap: %q, %v; want the new model", got, err) + } +} + +func TestSwap_RefusedWhenWeDoNotOwnTheServer(t *testing.T) { + // NewLLMPhraserAt points at a shared server the eval harness owns. Swapping + // there would kill a server another process depends on. + p := NewLLMPhraserAt("http://127.0.0.1:1/", DefaultConfig("/m/old.gguf")) + if _, err := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/new.gguf"}); !errors.Is(err, ErrSwapNotOwned) { + t.Fatalf("Swap on a borrowed server = %v; want ErrSwapNotOwned", err) + } +} + +func TestSwap_SameModelIsANoOp(t *testing.T) { + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + launchesBefore := fl.launch + + res, err := p.Swap(context.Background(), SwapSpec{ModelPath: "/m/old.gguf"}) + if err != nil { + t.Fatalf("Swap to the live model: %v", err) + } + if res.Model != "old" { + t.Errorf("res.Model = %q; want old", res.Model) + } + if fl.launch != launchesBefore { + t.Errorf("%d extra launches; swapping to the live model must not reload weights", fl.launch-launchesBefore) + } +} + +func TestSwap_EmptyModelPathRefused(t *testing.T) { + fl := &fakeFleet{models: map[string]string{"/m/old.gguf": "old"}} + p := newSwapPhraser(t, fl, "/m/old.gguf") + if _, err := p.Swap(context.Background(), SwapSpec{}); err == nil { + t.Fatal("Swap with no model path returned nil error") + } +}