From c65d9fb4736bec337f9d7b243117f416ecaa034c Mon Sep 17 00:00:00 2001 From: yetone Date: Sat, 10 Oct 2026 09:07:44 +0800 Subject: [PATCH] codex: magpie-models.json keeps the keys the user added by hand to a model, and says supports_reasoning_summaries itself, so Codex before 0.145 sends the effort and shows thinking (heng91, #1450) What was wrong: every write of ~/.codex/magpie-models.json rendered the catalog from magpie's models alone, so a key the user had put into an entry was gone after the next model switch, catalog sync (Refresh, a provider added, a gateway start) or a switch to Codex's own default, which also deleted the file outright. The key heng91 added, supports_reasoning_summaries, is one Codex before 0.145 needs: its client.rs build_reasoning sends no `reasoning` (no effort, no summary, so no thinking shown) for a model whose entry doesn't say true, until openai/codex#32206 (first in 0.145). 0.144 also refuses to load a catalog an entry of which lacks the key at all. What changed: - codexcat: every entry says supports_reasoning_summaries, true for a model with effort levels, false otherwise. Later Codex ignore it. - codexcat.Keep: before a write, the keys of an entry on disk magpie doesn't write are carried over to the entry of the same slug, and a true of supports_reasoning_summaries stays over magpie's false. A key magpie writes keeps magpie's value; a model magpie no longer serves drops out; an unreadable file keeps nothing. - agent/codex.go: the write when magpie becomes the provider and the catalog sync go through Keep; the three places that removed the file remove it only when nothing of the user's is in it. Run: - New tests TestCodexCatalogKeepsWhatTheUserAdded, TestCodexCatalogRemovedWhenNothingAdded (agent, from the file magpie writes, edited by hand) and TestCodexCatalogReasoningSummaries, TestKeepUserKeys (codexcat). Each fails on origin/main's code. - Real Codex in a sandbox CODEX_HOME against a capture server: 0.144.0 fails to load the old catalog ("missing field supports_reasoning_summaries"), loads the new one and sends reasoning {effort: high, summary: auto}; with the user's keys kept it sends summary "detailed". 0.162.0 loads both and sends the same. - gofmt -l clean; go vet, GOOS=linux and GOOS=windows go vet of ./internal/agent ./internal/codexcat; linux and windows builds; go test -tags nogui ./... under a temp HOME, all ok. --- internal/agent/codex.go | 31 +++++- internal/agent/codex_catalog_kept_test.go | 113 ++++++++++++++++++++++ internal/codexcat/codexcat.go | 97 +++++++++++++++++++ internal/codexcat/summaries_test.go | 99 +++++++++++++++++++ 4 files changed, 335 insertions(+), 5 deletions(-) create mode 100644 internal/agent/codex_catalog_kept_test.go create mode 100644 internal/codexcat/summaries_test.go diff --git a/internal/agent/codex.go b/internal/agent/codex.go index 0143b02a..3c59c18a 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -130,6 +130,27 @@ func codexIn(at place) *Agent { } return at.spell == nil && !filepath.IsAbs(c) && filepath.Clean(filepath.Join(dir, c)) == filepath.Clean(catalogPath) } + // ownList is magpie's model list for Codex as written to catalogPath: + // what the user added by hand to the list there (a key magpie doesn't + // write, supports_reasoning_summaries on a model magpie gave none) is + // kept, not written over by the next model switch or sync (#1450) + ownList := func() []byte { + cur, _ := edit.Read(catalogPath) + return codexcat.Keep(cur, codexcat.Catalog(magpieModels("codex"))) + } + // dropList takes the list away as magpie steps out as Codex's + // provider; one holding what the user added stays, which the config + // no longer names, for ownList to keep when magpie is back + dropList := func() { + cur, err := edit.Read(catalogPath) + if err != nil { + return + } + b := codexcat.Catalog(magpieModels("codex")) + if string(codexcat.Keep(cur, b)) == string(b) { + os.Remove(catalogPath) + } + } asProvider := func() bool { return get("model_provider") == magpieID } viaBase := func() bool { return isCodexGatewayOn(get("openai_base_url"), at.host()) } routed := func() bool { return asProvider() || viaBase() } @@ -241,7 +262,7 @@ func codexIn(at place) *Agent { if err := edit.DelTOMLTop(path, "model_provider", "model_catalog_json"); err != nil { return err } - os.Remove(catalogPath) + dropList() return nil } // isCCSwitchMirror reports whether a model_providers table is CC Switch's @@ -634,7 +655,7 @@ func codexIn(at place) *Agent { if err := edit.DelTOMLTop(path, "model", "model_provider", "model_catalog_json"); err != nil { return err } - os.Remove(catalogPath) + dropList() forget(at.key("codex.model"), at.key("codex.effort"), at.key("codex.provider"), at.key("codex.catalog"), at.key("codex.out"), at.key("codex.joined"), at.key("codex.beside")) return nil } @@ -700,7 +721,7 @@ func codexIn(at place) *Agent { if err := putProvider(); err != nil { return err } - if err := edit.WriteAtomic(catalogPath, codexcat.Catalog(magpieModels("codex"))); err != nil { + if err := edit.WriteAtomic(catalogPath, ownList()); err != nil { return err } // a thread started on Codex's built-in provider stays on it when @@ -771,7 +792,7 @@ func codexIn(at place) *Agent { if err != nil { return err } - os.Remove(catalogPath) + dropList() forget(at.key("codex.model"), at.key("codex.effort"), at.key("codex.provider"), at.key("codex.catalog")) return settle() }, @@ -882,7 +903,7 @@ func codexIn(at place) *Agent { return err } } - b := codexcat.Catalog(magpieModels("codex")) + b := ownList() if cur, _ := edit.Read(catalogPath); string(cur) != string(b) { if err := edit.WriteAtomic(catalogPath, b); err != nil { return err diff --git a/internal/agent/codex_catalog_kept_test.go b/internal/agent/codex_catalog_kept_test.go new file mode 100644 index 00000000..e3ac541b --- /dev/null +++ b/internal/agent/codex_catalog_kept_test.go @@ -0,0 +1,113 @@ +package agent + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/yetone/magpie/internal/provider" +) + +// What the user adds by hand to the model list magpie writes for Codex +// stays through the next model switch, sync and stepping out (#1450: +// supports_reasoning_summaries, put in for Codex to show a model's +// thinking, was gone after the next change). A key magpie writes, or took +// out on purpose (multi_agent_version with the V1 setting off), is +// magpie's. +func TestCodexCatalogKeepsWhatTheUserAdded(t *testing.T) { + home, _ := codexHome(t, "", "") + list := filepath.Join(home, ".codex", "magpie-models.json") + cx := codex(home) + if err := cx.Fields[0].Set("fake/m1"); err != nil { + t.Fatal(err) + } + // the file as magpie wrote it, edited by hand: two keys on fake/m1's + // entry, and a version magpie no longer writes + var cat map[string]any + if err := json.Unmarshal([]byte(readFile(list)), &cat); err != nil { + t.Fatal(err) + } + e := cat["models"].([]any)[0].(map[string]any) + if e["slug"] != "fake/m1" { + t.Fatalf("first entry: %v", e["slug"]) + } + e["supports_reasoning_summaries"] = true + e["default_reasoning_summary"] = "detailed" + e["multi_agent_version"] = "v1" + b, _ := json.MarshalIndent(cat, "", " ") + writeFile(t, list, string(b)) + + has := func(when string) { + t.Helper() + var got struct { + Models []map[string]any `json:"models"` + } + if err := json.Unmarshal([]byte(readFile(list)), &got); err != nil { + t.Fatalf("%s: %v", when, err) + } + var m1 map[string]any + for _, m := range got.Models { + if m["slug"] == "fake/m1" { + m1 = m + } + } + if m1 == nil || m1["supports_reasoning_summaries"] != true || m1["default_reasoning_summary"] != "detailed" { + t.Errorf("%s: the user's keys are gone: %v %v", when, m1["supports_reasoning_summaries"], m1["default_reasoning_summary"]) + } + if _, ok := m1["multi_agent_version"]; ok { + t.Errorf("%s: a version magpie no longer writes stayed", when) + } + if m1["base_instructions"] == nil || m1["supports_parallel_tool_calls"] != true { + t.Errorf("%s: magpie's own keys are gone", when) + } + } + + // a provider added: the sync writes the list again + if err := provider.Save(provider.Provider{ID: "added", Name: "Added", Key: "k", Chat: "http://127.0.0.1:1/v1", Models: []string{"m2"}}); err != nil { + t.Fatal(err) + } + SyncCatalog() + if !strings.Contains(readFile(list), `"added/m2"`) { + t.Fatal("sync didn't write the new model") + } + has("after a sync") + + if err := cx.Fields[0].Set("added/m2"); err != nil { + t.Fatal(err) + } + has("after a model switch") + + // Codex's own default: magpie steps out, and the list the user added + // to stays for when magpie is back + if err := cx.Fields[0].Set(""); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(list); err != nil { + t.Fatalf("the list the user added to was removed: %v", err) + } + if err := cx.Fields[0].Set("fake/m1"); err != nil { + t.Fatal(err) + } + has("after stepping out and back") +} + +// A list holding nothing of the user's still goes as magpie steps out. +func TestCodexCatalogRemovedWhenNothingAdded(t *testing.T) { + home, _ := codexHome(t, "", "") + list := filepath.Join(home, ".codex", "magpie-models.json") + cx := codex(home) + if err := cx.Fields[0].Set("fake/m1"); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(list); err != nil { + t.Fatal(err) + } + if err := cx.Fields[0].Set(""); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(list); err == nil { + t.Error("magpie's list stayed after it stepped out") + } +} diff --git a/internal/codexcat/codexcat.go b/internal/codexcat/codexcat.go index ac1d654c..e8da9dee 100644 --- a/internal/codexcat/codexcat.go +++ b/internal/codexcat/codexcat.go @@ -4,12 +4,14 @@ package codexcat import ( + "bytes" "crypto/sha256" _ "embed" "encoding/hex" "encoding/json" "os" "path/filepath" + "reflect" "regexp" "slices" "strings" @@ -120,6 +122,16 @@ type model struct { // fails to load); later Codex ask for parallel calls whatever it // says, so it says what they do. Parallel bool `json:"supports_parallel_tool_calls"` + // Required by Codex before 0.145: without it the whole catalog fails + // to load ("missing field `supports_reasoning_summaries`", tried on + // 0.144.0), and it sends a request's reasoning parameters โ€” the effort + // picked, and the summary it shows as the model's thinking โ€” only for + // a model whose entry says true (codex-rs client.rs build_reasoning, + // until openai/codex#32206), so the user had to add it by hand + // (#1450). Later Codex send them always and ignore the field. True for + // a model with effort levels; a true the user put in for another stays + // (Keep). + Summaries bool `json:"supports_reasoning_summaries" keep:"true"` // "v1" only with settings.CodexAgentsV1, on an OpenAI model's // entry (see V1); "v2" on a model offering Ultra that no ChatGPT // account answers for (catalog.Model.AgentsV2), as Codex's own @@ -203,12 +215,97 @@ func Entries(ms []catalog.Model, after int) []any { if len(m.Efforts) > 0 { d := DefaultEffort(m.Efforts) e.DefaultEffort = &d + e.Summaries = true } entries = append(entries, &e) } return entries } +// owned are the keys of an entry magpie writes, or leaves out on purpose: +// every field of model, and the two ownEntry takes out of Codex's own. +// keepTrue are those of them where a true the user set by hand stays over +// magpie's false (keep:"true"). +var owned, keepTrue = func() (own, yes map[string]bool) { + own = map[string]bool{"availability_nux": true, "upgrade": true} + yes = map[string]bool{} + t := reflect.TypeFor[model]() + for i := range t.NumField() { + f := t.Field(i) + name, _, _ := strings.Cut(f.Tag.Get("json"), ",") + if name == "" { + continue + } + own[name] = true + if f.Tag.Get("keep") == "true" { + yes[name] = true + } + } + return own, yes +}() + +// Keep is the catalog b, as Catalog renders it, with what the user added by +// hand to the entries of cur, the catalog on disk magpie is about to write +// over: a key of an entry of the same slug that magpie doesn't own, and a +// true where magpie says false of a keepTrue key โ€” supports_reasoning_ +// summaries, put in for Codex to send a model's effort and show its +// thinking (#1450). Another key magpie writes takes magpie's value; the +// entry of a model magpie no longer serves goes. b as it is when there is +// nothing to keep, or cur isn't a catalog. +func Keep(cur, b []byte) []byte { + type list struct { + Models []map[string]any `json:"models"` + } + read := func(raw []byte, l *list) error { + d := json.NewDecoder(bytes.NewReader(raw)) + d.UseNumber() + return d.Decode(l) + } + var was list + if len(cur) == 0 || read(cur, &was) != nil { + return b + } + add := map[string]map[string]any{} + for _, e := range was.Models { + slug, _ := e["slug"].(string) + if slug == "" { + continue + } + for k, v := range e { + if !owned[k] || keepTrue[k] && v == true { + if add[slug] == nil { + add[slug] = map[string]any{} + } + add[slug][k] = v + } + } + } + if len(add) == 0 { + return b + } + var now list + if read(b, &now) != nil { + return b + } + kept := false + for _, e := range now.Models { + slug, _ := e["slug"].(string) + for k, v := range add[slug] { + if was, ok := e[k]; !ok || keepTrue[k] && was != true { + e[k], kept = v, true + } + } + } + if !kept { + return b + } + out, err := json.MarshalIndent(now, "", " ") + if err != nil { + return b + } + return out +} + // datedSuffix is a snapshot's date after a model's id (-2026-09-14, // -20260914). var datedSuffix = regexp.MustCompile(`-(\d{4}-\d{2}-\d{2}|\d{8})$`) diff --git a/internal/codexcat/summaries_test.go b/internal/codexcat/summaries_test.go new file mode 100644 index 00000000..7944996a --- /dev/null +++ b/internal/codexcat/summaries_test.go @@ -0,0 +1,99 @@ +package codexcat + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/yetone/magpie/internal/catalog" +) + +// Codex before 0.145 loads no catalog an entry of which lacks +// supports_reasoning_summaries, and sends the effort and asks for the +// thinking summary only for a model whose entry says true (#1450): every +// entry says it, true for a model with effort levels. +func TestCodexCatalogReasoningSummaries(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("USERPROFILE", os.Getenv("HOME")) + var got struct { + Models []map[string]any `json:"models"` + } + json.Unmarshal(Catalog([]catalog.Model{ + {ID: "deepseek/deepseek-v4", Name: "v4", Efforts: []string{"high", "max"}}, + {ID: "fake/m1", Name: "m1"}, + }), &got) + if len(got.Models) != 2 { + t.Fatalf("%v", got) + } + if got.Models[0]["supports_reasoning_summaries"] != true { + t.Errorf("a model with efforts: %v", got.Models[0]["supports_reasoning_summaries"]) + } + if v, ok := got.Models[1]["supports_reasoning_summaries"]; !ok || v != false { + t.Errorf("a model without efforts: %v %v", v, ok) + } +} + +// Keep carries over what the user put in by hand, Codex's own entries too, +// and a true of supports_reasoning_summaries where magpie says false; of +// the rest magpie owns, nothing. +func TestKeepUserKeys(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + os.MkdirAll(filepath.Join(home, ".codex"), 0o755) + os.WriteFile(filepath.Join(home, ".codex", "models_cache.json"), []byte(`{"models":[ + {"slug":"gpt-5.5","display_name":"GPT-5.5","priority":3,"visibility":"list","context_window":272000,"supports_reasoning_summaries":true}]}`), 0o644) + ms := []catalog.Model{ + {ID: "fake/m1", Name: "m1"}, + {ID: "codex/gpt-5.5", Name: "GPT-5.5 ยท Codex", Efforts: []string{"low", "high"}}, + } + b := Catalog(ms) + if got := Keep(nil, b); string(got) != string(b) { + t.Error("no file: changed") + } + if got := Keep([]byte("{not json"), b); string(got) != string(b) { + t.Error("a file that doesn't parse: changed") + } + if got := Keep(b, b); string(got) != string(b) { + t.Error("magpie's own file: changed") + } + + cur := `{"models":[ + {"slug":"fake/m1","supports_reasoning_summaries":true,"x_note":{"by":"me"},"max_output_tokens":128000, + "context_window":1,"auto_review_model_override":"old/one","shell_type":"local"}, + {"slug":"codex/gpt-5.5","supports_reasoning_summaries":false,"upgrade":{"model":"gpt-6"},"x_mine":1}, + {"slug":"gone/model","x_note":"left"}]}` + var got struct { + Models []map[string]any `json:"models"` + } + d := json.NewDecoder(strings.NewReader(string(Keep([]byte(cur), b)))) + d.UseNumber() + if err := d.Decode(&got); err != nil || len(got.Models) != 2 { + t.Fatalf("%v %v", err, got) + } + m1, own := got.Models[0], got.Models[1] + if m1["supports_reasoning_summaries"] != true || m1["max_output_tokens"] != json.Number("128000") { + t.Errorf("user's keys: %v %v", m1["supports_reasoning_summaries"], m1["max_output_tokens"]) + } + if n, _ := m1["x_note"].(map[string]any); n["by"] != "me" { + t.Errorf("user's object: %v", m1["x_note"]) + } + if _, ok := m1["context_window"]; ok { + t.Error("a window magpie left out came back") + } + if _, ok := m1["auto_review_model_override"]; ok { + t.Error("an auto-review model magpie took out came back") + } + if m1["shell_type"] != "unified_exec" { + t.Errorf("a key magpie writes: %v", m1["shell_type"]) + } + // Codex's own entry: its cache's value is magpie's; a key of the user's stays + if own["supports_reasoning_summaries"] != true || own["x_mine"] != json.Number("1") { + t.Errorf("own entry: %v %v", own["supports_reasoning_summaries"], own["x_mine"]) + } + if _, ok := own["upgrade"]; ok { + t.Error("upgrade prompt came back") + } +}