diff --git a/internal/agent/agents.go b/internal/agent/agents.go index b9d336ea..a34d0328 100644 --- a/internal/agent/agents.go +++ b/internal/agent/agents.go @@ -578,7 +578,7 @@ func openCodeLike(at place, id, name, icon, bin, dir, auth string, ua []string, break } } - provider := func() any { return magpieProviderJSONAt("opencode", id, at.gw()) } + provider := theirsKept(path, "provider."+magpieID, func() any { return magpieProviderJSONAt("opencode", id, at.gw()) }, "models") opts := func(key string) func(map[string]string) []Option { return func(cur map[string]string) []Option { return append(ownOptions(auth, cur[key]), viaMagpie(id, magpieID+"/")...) @@ -654,7 +654,8 @@ func openCodeRefAt(at place, path, id, v string) (string, error) { if own := ownGatewayProvider(path, ref, at.v1()); own != "" { return own + "/" + ref, nil } - return v, edit.SetJSON(path, edit.KV{Path: "provider." + magpieID, Value: magpieProviderJSONAt("opencode", id, at.gw())}) + provider := theirsKept(path, "provider."+magpieID, func() any { return magpieProviderJSONAt("opencode", id, at.gw()) }, "models") + return v, edit.SetJSON(path, edit.KV{Path: "provider." + magpieID, Value: provider()}) } // ownGatewayProvider is the provider in an OpenCode config, other than @@ -797,8 +798,9 @@ func piLike(at place, id, name, dir string) *Agent { pair := pairSet(set, "defaultProvider", "defaultModel") // the model a new session starts on, as the model field shows it startup := func() string { return piStartup(path, pairGet(get, "defaultProvider", "defaultModel")()) } + block := theirsKept(modelsPath, "providers."+magpieID, func() any { return magpieProviderJSONAt("pi", id, at.gw()) }, "models") writeMagpie := func() error { - return edit.SetJSON(modelsPath, edit.KV{Path: "providers." + magpieID, Value: magpieProviderJSONAt("pi", id, at.gw())}) + return edit.SetJSON(modelsPath, edit.KV{Path: "providers." + magpieID, Value: block()}) } return &Agent{ ID: id, Name: name, Icon: id, Bin: id, Dir: dir, Path: path, Spelled: prefixed, @@ -810,7 +812,7 @@ func piLike(at place, id, name, dir string) *Agent { "baseUrl", at.v1(), "apiKey", at.gwKey()) }, Sync: func() error { - return syncJSON(modelsPath, "providers."+magpieID, func() any { return magpieProviderJSONAt("pi", id, at.gw()) }) + return syncJSON(modelsPath, "providers."+magpieID, block) }, Fields: []Field{ { @@ -1078,7 +1080,7 @@ func crushIn(at place) *Agent { // crushAt is Crush with its config at path and its data file at data, // reaching the gateway as at does. func crushAt(at place, path, data string) *Agent { - provider := func() any { return magpieProviderJSONAt("crush", "crush", at.gw()) } + provider := theirsKept(path, "providers."+magpieID, func() any { return magpieProviderJSONAt("crush", "crush", at.gw()) }, "models") get := func(k string) (string, bool) { return edit.GetJSON(path, k) } set := func(kvs ...edit.KV) error { return edit.SetJSON(path, kvs...) } pick := func(k string) (string, bool) { diff --git a/internal/agent/aside_native.go b/internal/agent/aside_native.go index b5f6a94c..0349c7c6 100644 --- a/internal/agent/aside_native.go +++ b/internal/agent/aside_native.go @@ -328,9 +328,9 @@ func (c *asideConnection) connectLocked() error { return fmt.Errorf("%s", detail) } if status == "connected" { - return syncJSON(c.models, "providers.magpie", c.block) + return syncJSON(c.models, "providers.magpie", c.kept()) } - return edit.SetJSON(c.models, edit.KV{Path: "providers.magpie", Value: c.block()}) + return edit.SetJSON(c.models, edit.KV{Path: "providers.magpie", Value: c.kept()()}) } func (c *asideConnection) sync() error { @@ -340,7 +340,13 @@ func (c *asideConnection) sync() error { if status != "connected" { return nil } - return syncJSON(c.models, "providers.magpie", c.block) + return syncJSON(c.models, "providers.magpie", c.kept()) +} + +// kept is the block with what else Aside's providers.magpie holds kept +// (theirsKept). +func (c *asideConnection) kept() func() any { + return theirsKept(c.models, "providers.magpie", c.block, "models") } func (c *asideConnection) state() NativeState { diff --git a/internal/agent/commandcode.go b/internal/agent/commandcode.go index 6ffbe925..03fcfe0e 100644 --- a/internal/agent/commandcode.go +++ b/internal/agent/commandcode.go @@ -43,7 +43,7 @@ func commandCodeIn(at place) *Agent { UA: []string{"command-code", "commandcode"}, Bin: "command-code", Dir: dir, Path: path, Sync: func() error { - return syncJSON(providers, "provider."+magpieID, func() any { return ccProviderJSON() }) + return syncJSON(providers, "provider."+magpieID, theirsKept(providers, "provider."+magpieID, ccProviderJSON, "models")) }, Notice: func() string { notes := []string{"Command Code wants its own sign-in (cmd login) even for models through magpie."} @@ -77,7 +77,7 @@ func commandCodeIn(at place) *Agent { return dropMagpie() } if ref, ok := strings.CutPrefix(v, magpieID+"/"); ok && isMagpie(ref) { - if err := edit.SetJSON(providers, edit.KV{Path: "provider." + magpieID, Value: ccProviderJSON()}); err != nil { + if err := edit.SetJSON(providers, edit.KV{Path: "provider." + magpieID, Value: theirsKept(providers, "provider."+magpieID, ccProviderJSON, "models")()}); err != nil { return err } return edit.SetJSON(path, edit.KV{Path: "model", Value: v}, edit.KV{Path: "modelProvider", Value: magpieID}) diff --git a/internal/agent/openchamber.go b/internal/agent/openchamber.go index 3de30425..87f922ed 100644 --- a/internal/agent/openchamber.go +++ b/internal/agent/openchamber.go @@ -165,7 +165,7 @@ func openChamber(home, cfg string) *Agent { oc := opencode(home, cfg) ocPath := oc.Path auth := filepath.Join(home, ".local", "share", "opencode", "auth.json") - provider := func() any { return magpieProviderJSONFor("opencode", "opencode") } + provider := theirsKept(ocPath, "provider."+magpieID, func() any { return magpieProviderJSONFor("opencode", "opencode") }, "models") opts := func(key string) func(map[string]string) []Option { return func(cur map[string]string) []Option { return append(ownOptions(auth, cur[key]), viaMagpie("opencode", magpieID+"/")...) diff --git a/internal/agent/pencil.go b/internal/agent/pencil.go index 47d0ee51..edc0863b 100644 --- a/internal/agent/pencil.go +++ b/internal/agent/pencil.go @@ -50,7 +50,7 @@ func pencil(home string) *Agent { return "" }, Sync: func() error { - return syncJSON(path, key, func() any { return pencilProviderJSON(gateway.URL()) }) + return syncJSON(path, key, theirsKept(path, key, func() any { return pencilProviderJSON(gateway.URL()) }, "models")) }, Fields: []Field{{ Key: "provider", Label: "provider", @@ -64,7 +64,7 @@ func pencil(home string) *Agent { if v == "" { return edit.DelJSON(path, key) } - return edit.SetJSON(path, edit.KV{Path: key, Value: pencilProviderJSON(gateway.URL())}) + return edit.SetJSON(path, edit.KV{Path: key, Value: theirsKept(path, key, func() any { return pencilProviderJSON(gateway.URL()) }, "models")()}) }, Options: func(map[string]string) []Option { return []Option{{Value: magpieID, Label: "magpie", Icon: "magpie", Note: "every magpie model in Pencil's model picker"}} diff --git a/internal/agent/sync.go b/internal/agent/sync.go index 62323a0c..47d03fae 100644 --- a/internal/agent/sync.go +++ b/internal/agent/sync.go @@ -5,6 +5,7 @@ import ( "encoding/json" "io" "reflect" + "slices" "strings" "sync" @@ -76,6 +77,56 @@ func syncJSON(path, key string, value func() any) error { return edit.SetJSON(path, edit.KV{Path: key, Value: v}) } +// theirsKept is value, magpie's provider block for the agent, with what +// else the block at key of the JSON file holds kept: a key magpie doesn't +// write there, at any depth, is the user's or another tool's (Pi's +// providers.magpie.compat.sendSessionAffinityHeaders, which +// pi-cache-optimizer adds, #1103) and stays. The keys named in whole (the +// model list) are magpie's alone, so a model gone from the catalog leaves +// the agent's list. +func theirsKept(path, key string, value func() any, whole ...string) func() any { + return func() any { + v := value() + cur, ok := edit.GetJSON(path, key) + if !ok { + return v + } + var have, want map[string]json.RawMessage + b, err := json.Marshal(v) + if err != nil || json.Unmarshal([]byte(cur), &have) != nil || have == nil || json.Unmarshal(b, &want) != nil || want == nil { + return v + } + return mergeTheirs(have, want, whole) + } +} + +// mergeTheirs is want with each key of have that want lacks, objects in +// both merged alike; a key named in whole is want's as it is. +func mergeTheirs(have, want map[string]json.RawMessage, whole []string) map[string]json.RawMessage { + out := make(map[string]json.RawMessage, len(have)+len(want)) + for k, w := range want { + out[k] = w + } + for k, h := range have { + w, ok := want[k] + if !ok { + out[k] = h + continue + } + if slices.Contains(whole, k) { + continue + } + var hm, wm map[string]json.RawMessage + if json.Unmarshal(h, &hm) != nil || hm == nil || json.Unmarshal(w, &wm) != nil || wm == nil { + continue + } + if b, err := json.Marshal(mergeTheirs(hm, wm, nil)); err == nil { + out[k] = b + } + } + return out +} + // syncJSONInOrder is syncJSON for a block whose keys' order the agent reads // (OpenCode lists a model's variants in theirs): one that says the same in // another order is rewritten too, as an older magpie wrote the variants diff --git a/internal/agent/sync_keep_test.go b/internal/agent/sync_keep_test.go new file mode 100644 index 00000000..1a76b2dd --- /dev/null +++ b/internal/agent/sync_keep_test.go @@ -0,0 +1,120 @@ +package agent + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// providerAt is the object at providers. (or provider.) of a JSON +// file, decoded. +func providerAt(t *testing.T, path, top string) map[string]any { + t.Helper() + var f map[string]json.RawMessage + var ps map[string]map[string]any + if err := json.Unmarshal([]byte(readFile(path)), &f); err != nil || json.Unmarshal(f[top], &ps) != nil { + t.Fatalf("%s: %v\n%s", path, err, readFile(path)) + } + return ps[magpieID] +} + +// #1103: pi-cache-optimizer's /cache-optimizer fix adds +// providers.magpie.compat.sendSessionAffinityHeaders to Pi's models.json. +// A catalog sync, and a pick of a magpie model in Pi, keep it; the model +// list is still magpie's, so a model gone from the catalog leaves it, and +// a sync with nothing new writes nothing. +func TestPiKeepsTheirKeysInMagpiesProvider(t *testing.T) { + home := syncHome(t) + piModels := filepath.Join(home, ".pi", "agent", "models.json") + // the block as magpie wrote it, with the optimizer's key added + writeFile(t, piModels, `{ + "providers": { + "magpie": { + "api": "openai-completions", + "apiKey": "magpie", + "baseUrl": "http://127.0.0.1:3425/v1", + "compat": { + "sendSessionAffinityHeaders": true + }, + "models": [ + { + "id": "gone/old-model", + "name": "Old" + } + ], + "name": "magpie" + } + } +} +`) + SyncCatalog() + p := providerAt(t, piModels, "providers") + compat, _ := p["compat"].(map[string]any) + if compat["sendSessionAffinityHeaders"] != true { + t.Fatalf("compat.sendSessionAffinityHeaders dropped by the sync:\n%s", readFile(piModels)) + } + ms, _ := json.Marshal(p["models"]) + if strings.Contains(string(ms), "gone/old-model") || !strings.Contains(string(ms), "relay/glm-4.6") { + t.Fatalf("models not magpie's list: %s", ms) + } + + // synced already: the next sync writes nothing + st, _ := os.Stat(piModels) + old := st.ModTime().Add(-1e12) + os.Chtimes(piModels, old, old) + SyncCatalog() + if now, _ := os.Stat(piModels); !now.ModTime().Equal(old) { + t.Errorf("models.json rewritten though nothing changed:\n%s", readFile(piModels)) + } + + // picking a magpie model in Pi writes the block too + if err := pi(home).Fields[0].Set("magpie/relay/glm-4.6"); err != nil { + t.Fatal(err) + } + compat, _ = providerAt(t, piModels, "providers")["compat"].(map[string]any) + if compat["sendSessionAffinityHeaders"] != true { + t.Fatalf("compat.sendSessionAffinityHeaders dropped by a pick:\n%s", readFile(piModels)) + } +} + +// The same for OpenCode's provider.magpie (an options key of the user's +// own) and Crush's providers.magpie. +func TestOpenCodeAndCrushKeepTheirKeysInMagpiesProvider(t *testing.T) { + home := syncHome(t) + cfg := filepath.Join(home, ".config", "opencode", "opencode.json") + writeFile(t, cfg, `{"model":"magpie/relay/glm-4.6","provider":{"magpie":{"name":"magpie","npm":"@ai-sdk/openai-compatible","options":{"baseURL":"http://127.0.0.1:3425/v1","apiKey":"magpie","setCacheKey":true},"models":{"gone/old-model":{"name":"Old"}}}}}`) + oc := opencode(home, filepath.Join(home, ".config")) + if err := oc.Sync(); err != nil { + t.Fatal(err) + } + p := providerAt(t, cfg, "provider") + opts, _ := p["options"].(map[string]any) + ms, _ := p["models"].(map[string]any) + if opts["setCacheKey"] != true || ms["gone/old-model"] != nil || ms["relay/glm-4.6"] == nil { + t.Fatalf("opencode sync:\n%s", readFile(cfg)) + } + if err := oc.Fields[0].Set("magpie/relay/glm-4.6"); err != nil { + t.Fatal(err) + } + if opts, _ := providerAt(t, cfg, "provider")["options"].(map[string]any); opts["setCacheKey"] != true { + t.Fatalf("opencode pick:\n%s", readFile(cfg)) + } + + cr := filepath.Join(home, ".config", "crush", "crush.json") + writeFile(t, cr, `{"providers":{"magpie":{"type":"openai","name":"magpie","base_url":"http://127.0.0.1:3425/v1","api_key":"magpie","extra_headers":{"X-Mine":"1"},"models":[]}}}`) + c := crushAt(here(home), cr, filepath.Join(home, ".local", "share", "crush", "crush.json")) + if err := c.Sync(); err != nil { + t.Fatal(err) + } + if h, _ := providerAt(t, cr, "providers")["extra_headers"].(map[string]any); h["X-Mine"] != "1" { + t.Fatalf("crush sync:\n%s", readFile(cr)) + } + if err := c.Fields[0].Set("magpie/relay/glm-4.6"); err != nil { + t.Fatal(err) + } + if h, _ := providerAt(t, cr, "providers")["extra_headers"].(map[string]any); h["X-Mine"] != "1" { + t.Fatalf("crush pick:\n%s", readFile(cr)) + } +}