diff --git a/docs/reference.md b/docs/reference.md index 6ea8b7f2..e3d3b40a 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -756,7 +756,12 @@ model (`opus`, `sonnet`…) removes them and restores whatever was there. **Codex** gets a `[model_providers.magpie]` table, `model_catalog_json` pointing at `~/.codex/magpie-models.json` (written from the catalog, so the models show in Codex's own list) and a valid `model`/`effort`; picking a -native model removes all of that. Your ChatGPT sign-in is never touched. +native model or disconnecting removes `model_provider` and the catalog. The +`[model_providers.magpie]` table stays, so a thread started on magpie can +still be opened. Codex won't load its config at all when `model_provider = +"magpie"` has no table ("Model provider `magpie` not found"). If another +tool leaves that state behind, magpie writes the table back the next time +it syncs. Your ChatGPT sign-in is never touched. Codex reads its model list at start-up, so restart it after a switch. **OpenCode, Pi, Crush** get a `magpie` provider entry and `magpie/provider/model`. diff --git a/internal/agent/applied.go b/internal/agent/applied.go index a4786362..fe9e7ec8 100644 --- a/internal/agent/applied.go +++ b/internal/agent/applied.go @@ -392,6 +392,15 @@ func (a *Agent) Pick(key, v string) error { // the provider and the whole catalog in, so the agent's own model list has // every one of magpie's models. An agent that can keep the model it is on // (Join) keeps it. An agent already connected is left as it is. +// NoModelsError is Connect's answer while magpie has no models to give +// the agent: no provider or subscription has been added yet. The GUI says +// it in the reader's language by its code (no_models). +type NoModelsError struct{ Agent string } + +func (e *NoModelsError) Error() string { + return "Add a provider or subscription in magpie first, then connect " + e.Agent +} + func (a *Agent) Connect() error { _, err := a.ConnectHow() return err @@ -474,6 +483,11 @@ func (a *Agent) connect() (Connection, error) { } } if f == nil { + // nothing to connect it to yet: no provider or subscription added, + // which said only that it can't be connected (Tystem on Discord) + if shown, hidden := provider.CatalogFor(a.ID); len(shown)+len(hidden) == 0 { + return Connection{}, &NoModelsError{Agent: a.Name} + } return Connection{}, fmt.Errorf("%s can't be connected to magpie", a.Name) } cur := connectWas(vals[f.Key], opts) diff --git a/internal/agent/codex.go b/internal/agent/codex.go index f9d3e1df..ea463940 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -48,7 +48,13 @@ import ( // The [model_providers.magpie] table stays once written. A thread keeps the // provider it was started on, and one started on magpie can't be opened // again without the table ("Model provider `magpie` not found"), whichever -// way Codex is routed now. +// way Codex is routed now. Codex looks model_provider up in the tables as +// it loads config.toml (codex-rs config: "Model provider `{id}` not +// found"), so `model_provider = "magpie"` without the table keeps it from +// loading its config at all, a new thread too: magpie never writes the one +// without the other, takes model_provider out whenever it steps out as the +// provider (disconnected, unwired, joined beside the sign-in), and Sync +// writes the table back when something else took it. // codexStandIn is the model Codex is set to use, when magpie is its // provider, for a model it names that magpie doesn't serve. Codex asks for @@ -163,6 +169,31 @@ func codexIn(at place) *Agent { t, err := edit.GetTOMLTable(path, "model_providers."+magpieID) return err == nil && t != nil } + // noProvider: the config was read and has no such table; one magpie + // can't read is told by Check as it is, and written to by nothing + noProvider := func() bool { + t, err := edit.GetTOMLTable(path, "model_providers."+magpieID) + return err == nil && t == nil + } + // namesMagpie: the config has Codex on magpie as its provider, at its + // top or in a profile. Codex then reads [model_providers.magpie] as it + // loads the config, before any thread: without the table it loads no + // config at all ("Model provider `magpie` not found"), the app and the + // CLI both, whatever the model. + namesMagpie := func() bool { + if asProvider() { + return true + } + names, _ := edit.TOMLTables(path) + for _, n := range names { + if strings.HasPrefix(n, "profiles.") { + if t, _ := edit.GetTOMLTable(path, n); t["model_provider"] == magpieID { + return true + } + } + } + return false + } // dropProvider takes magpie out as the provider Codex is on; the table // stays for the threads started on it dropProvider := func() error { @@ -678,6 +709,15 @@ func codexIn(at place) *Agent { Joined: joined, Routed: routed, Sync: func() error { + // model_provider = "magpie" left with its table gone (taken by + // another tool, or a hand edit), which keeps Codex from loading + // its config at all, connected to magpie or not (Tystem on + // Discord): the table is written back, as it stays once written + if namesMagpie() && noProvider() { + if err := putProvider(); err != nil { + return err + } + } if err := failover(); err != nil { return err } @@ -750,6 +790,9 @@ func codexIn(at place) *Agent { return nil }, Check: func() string { + if namesMagpie() && noProvider() { + return "Codex's config names magpie as its model_provider but has no [model_providers.magpie], so Codex can't load its config (\"Model provider `magpie` not found\"): magpie writes it back when it next syncs, or connect Codex again" + } if !wired() { return "" } diff --git a/internal/agent/codex_halfwired_test.go b/internal/agent/codex_halfwired_test.go new file mode 100644 index 00000000..c6e333db --- /dev/null +++ b/internal/agent/codex_halfwired_test.go @@ -0,0 +1,85 @@ +package agent + +import ( + "errors" + "strings" + "testing" + + "github.com/yetone/magpie/internal/edit" + "github.com/yetone/magpie/internal/provider" +) + +// Tystem on Discord: Codex wouldn't load its config ("Model provider +// `magpie` not found") with model_provider = "magpie" and model = +// "deepseek-flash" left in config.toml and no [model_providers.magpie], +// and magpie showed Codex as not connected. Codex refuses the whole config +// so, not just an old thread: Check says so, and Sync writes the table +// back, at the top and for a profile alike. +func TestCodexProviderWithoutTable(t *testing.T) { + for _, tc := range []struct{ name, config string }{ + {"top", "model_provider = \"magpie\"\nmodel = \"deepseek-flash\"\n"}, + {"profile", "profile = \"ds\"\n\n[profiles.ds]\nmodel_provider = \"magpie\"\nmodel = \"deepseek-flash\"\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + home, read := codexHome(t, "", tc.config) + cx := codex(home) + if msg := cx.Check(); !strings.Contains(msg, "[model_providers.magpie]") { + t.Fatalf("check: %q", msg) + } + if err := cx.Sync(); err != nil { + t.Fatal(err) + } + tb, err := edit.GetTOMLTable(cx.Path, "model_providers.magpie") + if err != nil || tb["base_url"] != here(home).v1() || tb["wire_api"] != "responses" || tb["experimental_bearer_token"] == "" { + t.Fatalf("table %v %v:\n%s", tb, err, read()) + } + if msg := cx.Check(); strings.Contains(msg, "[model_providers.magpie]") { + t.Fatalf("check after sync: %q", msg) + } + }) + } +} + +// Every way magpie steps out as Codex's provider takes model_provider out +// with it and leaves the table: never the half state above. +func TestCodexSteppingOutLeavesNoHalfState(t *testing.T) { + for _, tc := range []struct { + name string + out func(a *Agent) error + }{ + {"disconnect", func(a *Agent) error { return a.Fields[0].Set("") }}, + {"unwire", func(a *Agent) error { return a.Unwire() }}, + {"own model", func(a *Agent) error { return a.Fields[0].Set("gpt-5.5") }}, + } { + t.Run(tc.name, func(t *testing.T) { + home, read := codexHome(t, "", "model = \"gpt-5.5\"\n") + cx := codex(home) + if err := cx.Fields[0].Set("fake/m1"); err != nil { + t.Fatal(err) + } + if err := tc.out(cx); err != nil { + t.Fatal(err) + } + if p, _ := edit.GetTOMLTop(cx.Path, "model_provider"); p == magpieID { + t.Fatalf("model_provider left:\n%s", read()) + } + if tb, _ := edit.GetTOMLTable(cx.Path, "model_providers.magpie"); tb == nil { + t.Fatalf("table gone, for the threads started on magpie:\n%s", read()) + } + }) + } +} + +// Connecting with no provider or subscription in magpie says what to do, +// with a code the GUI says it by in the reader's language. +func TestConnectWithNoModels(t *testing.T) { + home, _ := codexHome(t, "", "model = \"gpt-5.5\"\n") + if err := provider.Delete("fake"); err != nil { + t.Fatal(err) + } + _, err := codex(home).ConnectHow() + var nm *NoModelsError + if !errors.As(err, &nm) || nm.Agent != "Codex" || !strings.Contains(err.Error(), "Add a provider or subscription in magpie first, then connect Codex") { + t.Fatalf("%v", err) + } +} diff --git a/internal/agent/codex_thread_test.go b/internal/agent/codex_thread_test.go index a1c64d92..fa4a15c6 100644 --- a/internal/agent/codex_thread_test.go +++ b/internal/agent/codex_thread_test.go @@ -84,7 +84,8 @@ func TestCodexProviderKeepsOwnBaseURL(t *testing.T) { // A Codex that an older magpie made its provider, with no base URL, gets // magpie's at the next sync; a provider set up by hand with a catalog of -// the user's own is not touched. +// the user's own keeps its keys, and only the table it names comes back +// (Codex loads no config while it is missing). func TestCodexSyncAddsBaseURLToProvider(t *testing.T) { home, read := codexHome(t, "", "") cx := codex(home) @@ -116,7 +117,7 @@ func TestCodexSyncAddsBaseURLToProvider(t *testing.T) { if err := codex(home).Sync(); err != nil { t.Fatal(err) } - if cfg = read(); cfg != mine { + if cfg = read(); !strings.HasPrefix(cfg, mine) || strings.Contains(cfg, "openai_base_url") || !strings.Contains(cfg, "[model_providers.magpie]") { t.Fatalf("hand-made provider changed:\n%s", cfg) } } diff --git a/internal/gui/agent_nomodels_test.go b/internal/gui/agent_nomodels_test.go new file mode 100644 index 00000000..37cffae3 --- /dev/null +++ b/internal/gui/agent_nomodels_test.go @@ -0,0 +1,32 @@ +package gui + +import ( + "encoding/json" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// Tystem on Discord: connecting Codex with no provider or subscription in +// magpie answered "Codex can't be connected to magpie", untranslated. The +// answer carries code no_models and the agent's name, which the GUI says +// in the reader's language. +func TestAgentConnectNoModels(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) + t.Setenv("CODEX_HOME", filepath.Join(home, ".codex")) + os.MkdirAll(filepath.Join(home, ".codex"), 0o755) + os.WriteFile(filepath.Join(home, ".codex", "config.toml"), []byte("model = \"gpt-5.5\"\n"), 0o600) + rec := httptest.NewRecorder() + Handler(nil, nil).ServeHTTP(rec, httptest.NewRequest("POST", "/api/agents/connect/codex", strings.NewReader("{}"))) + var out map[string]string + json.Unmarshal(rec.Body.Bytes(), &out) + if rec.Code != 400 || out["code"] != "no_models" || out["agent"] != "Codex" || !strings.Contains(out["error"], "Add a provider or subscription") { + t.Fatalf("%d %s", rec.Code, rec.Body) + } +} diff --git a/internal/gui/api.go b/internal/gui/api.go index 7c43ffcc..987c2f62 100644 --- a/internal/gui/api.go +++ b/internal/gui/api.go @@ -1548,6 +1548,11 @@ func fail(rw http.ResponseWriter, err error) { out["code"] = "runtime_unavailable" out["offline"] = string(unavailable.Offline) } + var noModels *agent.NoModelsError + if errors.As(err, &noModels) { + out["code"] = "no_models" + out["agent"] = noModels.Agent + } _ = json.NewEncoder(rw).Encode(out) } diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index 7b00d585..8646c5a7 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -104,6 +104,9 @@ async function api(path, body) { err.code = data.code; if (data.offline === "stage" || data.offline === "disconnect") err.offline = data.offline; } + // magpie has no models to connect an agent to yet: said in the + // reader's language + if (data?.code === "no_models") err.message = t("Add a provider or subscription in magpie first, then connect {agent}", { agent: data.agent }); if (data?.why) err.why = data.why; // a failed move's reason, said in the reader's language throw err; } diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index 26402d72..f90186ad 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -285,6 +285,7 @@ const I18N = { "Connected · pick magpie's models with {cmd} in {agent}": "已接入 · 在 {agent} 里用 {cmd} 选 magpie 的模型", "Connected · magpie's models are in {agent}'s model list": "已接入 · magpie 的模型在 {agent} 的模型列表里", "Connect {agent} to magpie": "将 {agent} 接入 magpie", + "Add a provider or subscription in magpie first, then connect {agent}": "请先在 magpie 中添加一个供应商或订阅，再接入 {agent}", "Connected · switch off to put back what {agent} had before magpie": "已接入 · 关掉即还原 {agent} 接入 magpie 之前的设置", "Switch on and {agent}'s model list gets magpie's models": "打开后，{agent} 的模型列表里就有 magpie 的模型", "{agent} is connected to magpie": "{agent} 已接入 magpie", @@ -3622,6 +3623,7 @@ const I18N = { "Connected · pick magpie's models with {cmd} in {agent}": "接続済み · {agent} で {cmd} を使って magpie のモデルを選択", "Connected · magpie's models are in {agent}'s model list": "接続済み · magpie のモデルは {agent} のモデル一覧にあります", "Connect {agent} to magpie": "{agent} を magpie に接続", + "Add a provider or subscription in magpie first, then connect {agent}": "先に magpie でプロバイダかサブスクリプションを追加してから、{agent} を接続してください", "Connected · switch off to put back what {agent} had before magpie": "接続済み · オフにすると {agent} を magpie 接続前の設定に戻します", "Switch on and {agent}'s model list gets magpie's models": "オンにすると {agent} のモデル一覧に magpie のモデルが加わります", "{agent} is connected to magpie": "{agent} は magpie に接続されています", @@ -6940,6 +6942,7 @@ const I18N = { "Connected · pick magpie's models with {cmd} in {agent}": "Verbunden · magpie-Modelle mit {cmd} in {agent} wählen", "Connected · magpie's models are in {agent}'s model list": "Verbunden · magpie-Modelle stehen in der Modellliste von {agent}", "Connect {agent} to magpie": "{agent} mit magpie verbinden", + "Add a provider or subscription in magpie first, then connect {agent}": "Fügen Sie zuerst in magpie einen Anbieter oder ein Abo hinzu und verbinden Sie dann {agent}", "Connected · switch off to put back what {agent} had before magpie": "Verbunden · ausschalten, um den Stand von {agent} vor magpie wiederherzustellen", "Switch on and {agent}'s model list gets magpie's models": "Einschalten, dann erhält die Modellliste von {agent} die magpie-Modelle", "{agent} is connected to magpie": "{agent} ist mit magpie verbunden", diff --git a/internal/gui/tests/agent-connect-no-models.test.cjs b/internal/gui/tests/agent-connect-no-models.test.cjs new file mode 100644 index 00000000..bfceb999 --- /dev/null +++ b/internal/gui/tests/agent-connect-no-models.test.cjs @@ -0,0 +1,74 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// Tystem on Discord: switching Codex on with no provider or subscription in +// magpie said only "Codex can't be connected to magpie", in English whatever +// the language. The server answers that case with code no_models, and the +// toast says what to do in the reader's language; the switch goes back off. +// No click moves the page. No backend, the API is faked here. +const assert = require("node:assert/strict"); +const fs = require("node:fs/promises"); +const path = require("node:path"); +const { test } = require("node:test"); +const { chromium, webkit } = require("playwright"); + +const assets = path.resolve(__dirname, "../assets"); +const options = [{ value: "gpt-5.4", label: "GPT-5.4" }, { value: "relay/m1", label: "m1", ref: "relay/m1", note: "Relay · via magpie" }]; +const state = { agents: [{ id: "codex", name: "Codex", icon: "generic", path: "/fixture/codex", fields: [{ key: "model", label: "model", value: "gpt-5.4", options }] }], profiles: [] }; + +function server(lang, posts) { + return async (route) => { + const req = route.request(), url = new URL(req.url()); + const json = (data, status = 200) => route.fulfill({ status, json: data }); + if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"${lang}",theme:"light",web:true};` }); + if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); + if (url.pathname === "/api/state") return json({ ...state, settings: { lang, theme: "light" } }); + if (url.pathname === "/api/agents/connect/codex") { + posts.push(url.pathname); + return json({ error: "Add a provider or subscription in magpie first, then connect Codex", code: "no_models", agent: "Codex" }, 400); + } + if (url.pathname === "/api/usage/quotas") return json([]); + if (url.pathname === "/api/groups") return json({ groups: [] }); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname === "/api/agents/cli") return json({ agents: {}, pending: false }); + if (url.pathname === "/api/providers") return json({ providers: [], presets: [], excluded: [], gateway: { running: true, window: true } }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)]; + await route.fulfill({ body: await fs.readFile(file), contentType }); + }; +} + +const said = { + en: "Add a provider or subscription in magpie first, then connect Codex", + zh: "请先在 magpie 中添加一个供应商或订阅，再接入 Codex", + ja: "先に magpie でプロバイダかサブスクリプションを追加してから、Codex を接続してください", + de: "Fügen Sie zuerst in magpie einen Anbieter oder ein Abo hinzu und verbinden Sie dann Codex", +}; +const sw = '.row.agent[data-id="codex"] .ag-conn'; + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + for (const lang of Object.keys(said)) { + test(`${engine} ${lang}: connecting with no models says to add one first`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + const page = await (await browser.newContext({ viewport: { width: 980, height: 520 }, reducedMotion: "reduce" })).newPage(); + page.setDefaultTimeout(5000); + const errors = [], posts = [], dialogs = []; + page.on("pageerror", (e) => errors.push(e.message)); + page.on("dialog", (d) => { dialogs.push(d.message()); d.dismiss(); }); + await page.route("**/*", server(lang, posts)); + await page.goto("http://magpie.test/?view=agents"); + await page.locator(sw).waitFor(); + const tops = () => page.evaluate(() => [document.scrollingElement.scrollTop, document.querySelector("#view-agents")?.scrollTop]); + const top = await tops(); + + await page.locator(sw).click(); + await page.waitForFunction((s) => document.querySelector("#status")?.textContent.includes(s), said[lang]); + assert.deepEqual(posts, ["/api/agents/connect/codex"]); + assert.equal(await page.locator(sw).getAttribute("aria-checked"), "false", "the switch goes back off"); + assert.ok(!(await page.locator(sw).isDisabled()), "and can be tried again"); + assert.deepEqual(await tops(), top, "no click moved the page"); + assert.deepEqual(dialogs, [], "no native dialog"); + assert.deepEqual(errors, []); + }); + } +}