diff --git a/internal/backup/backup.go b/internal/backup/backup.go index 58615ce4..f6704535 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -80,7 +80,17 @@ var ErrPassphrase = errors.New("wrong passphrase, or the file was changed") // variable or header that looks like one (their names stay). func Collect(keys bool, app string) (Bundle, error) { b := Bundle{Version: 1, Created: time.Now().UTC(), App: app, Keys: keys} - for _, p := range provider.Stored() { + // a providers.json that can't be read stops the backup: carried as no + // providers, it would take them all away where it is put back, or where + // sync mirrors it + stored, err := provider.Stored() + if err != nil { + return b, err + } + if b.Groups, err = provider.StoredGroups(); err != nil { + return b, err + } + for _, p := range stored { if !keys { p = withoutKeys(p) } @@ -96,7 +106,6 @@ func Collect(keys bool, app string) (Bundle, error) { } } } - b.Groups = provider.StoredGroups() searches := []provider.SearchAPI{} for _, a := range provider.StoredSearchAPIs() { if !keys { @@ -266,7 +275,11 @@ func Restore(b Bundle, parts Parts) (Result, error) { } } } - for _, p := range provider.Stored() { + stored, err := provider.Stored() + if err != nil { + return r, err + } + for _, p := range stored { if !p.Ready() && slices.ContainsFunc(b.Providers, func(q provider.Provider) bool { return q.ID == p.ID }) { r.NeedKey = append(r.NeedKey, p.Name) } diff --git a/internal/davsync/balance_token_test.go b/internal/davsync/balance_token_test.go index 97ffdfc2..1f8da903 100644 --- a/internal/davsync/balance_token_test.go +++ b/internal/davsync/balance_token_test.go @@ -117,7 +117,7 @@ func TestSyncBalanceToken(t *testing.T) { if serverKeys { localKey, localToken = "sk-a", "balance-a" } - ps := provider.Stored() + ps, _ := provider.Stored() if len(ps) != 1 || ps[0].Key != localKey || ps[0].BalanceToken != localToken { t.Fatalf("b after download: %+v", ps) } @@ -130,12 +130,12 @@ func TestSyncBalanceToken(t *testing.T) { if got := remote(); got.Keys != serverKeys || len(got.Providers) != 1 || got.Providers[0].Name != "Renamed" || got.Providers[0].Key != wantKey || got.Providers[0].BalanceToken != wantToken { t.Fatalf("after b's keyless upload: %+v", got) } - if ps := provider.Stored(); len(ps) != 1 || ps[0].BalanceToken != "balance-b-new" { + if ps, _ := provider.Stored(); len(ps) != 1 || ps[0].BalanceToken != "balance-b-new" { t.Fatalf("upload changed b's token: %+v", ps) } use(a) now() - if ps := provider.Stored(); len(ps) != 1 || ps[0].Name != "Renamed" || ps[0].Key != "sk-a" || ps[0].BalanceToken != "balance-a" { + if ps, _ := provider.Stored(); len(ps) != 1 || ps[0].Name != "Renamed" || ps[0].Key != "sk-a" || ps[0].BalanceToken != "balance-a" { t.Fatalf("a after download: %+v", ps) } }) diff --git a/internal/davsync/davsync_test.go b/internal/davsync/davsync_test.go index 0a62675f..fd540cef 100644 --- a/internal/davsync/davsync_test.go +++ b/internal/davsync/davsync_test.go @@ -101,7 +101,8 @@ func (c computer) use(t *testing.T) { func ids() []string { var out []string - for _, p := range provider.Stored() { + ps, _ := provider.Stored() + for _, p := range ps { out = append(out, p.ID+"="+p.Key) } slices.Sort(out) @@ -252,7 +253,7 @@ func TestSync(t *testing.T) { now(t) a.use(t) now(t) - if ps := provider.Stored(); len(ps) != 1 || ps[0].Name != "Kimi 2" || ps[0].Key != "k2" { + if ps, _ := provider.Stored(); len(ps) != 1 || ps[0].Name != "Kimi 2" || ps[0].Key != "k2" { t.Fatalf("a after c renamed: %+v", ps) } remote, _ := backup.Open(fake.files["/dav/magpie/magpie.magpie-backup"], "correct horse") diff --git a/internal/davsync/unreadable_test.go b/internal/davsync/unreadable_test.go new file mode 100644 index 00000000..b9521eb8 --- /dev/null +++ b/internal/davsync/unreadable_test.go @@ -0,0 +1,83 @@ +package davsync + +import ( + "bytes" + "context" + "errors" + "net/http/httptest" + "os" + "slices" + "testing" + + "github.com/yetone/magpie/internal/backup" + "github.com/yetone/magpie/internal/provider" +) + +// A providers.json that can't be read is never pushed as no providers: a +// sync stops on it, the server keeps the providers, and another computer +// mirroring the server keeps them too. +func TestSyncStopsOnUnreadableProviders(t *testing.T) { + fake := &fakeDAV{files: map[string][]byte{}, etags: map[string]string{}, dirs: map[string]bool{"/dav": true}} + srv := httptest.NewServer(fake) + defer srv.Close() + ctx := context.Background() + cfg := Config{URL: srv.URL + "/dav/", User: "me", Password: "pw", Passphrase: "correct horse", Keys: true} + + a, b := newComputer(t), newComputer(t) + b.use(t) + if err := Configure(cfg); err != nil { + t.Fatal(err) + } + a.use(t) + if err := provider.Save(provider.Provider{ID: "deepseek", Name: "DeepSeek", Chat: "https://api.deepseek.com/v1", Key: "k1"}); err != nil { + t.Fatal(err) + } + if err := Configure(cfg); err != nil { + t.Fatal(err) + } + if err := Now(ctx); err != nil { + t.Fatal(err) + } + b.use(t) + if err := Now(ctx); err != nil { + t.Fatal(err) + } + if got := ids(); !slices.Equal(got, []string{"deepseek=k1"}) { + t.Fatalf("b's providers: %v", got) + } + + // a's file is cut short + a.use(t) + whole, err := os.ReadFile(provider.Path()) + if err != nil { + t.Fatal(err) + } + broken := whole[:len(whole)/2] + if err := os.WriteFile(provider.Path(), broken, 0o600); err != nil { + t.Fatal(err) + } + puts := fake.puts + if err := Now(ctx); !errors.Is(err, provider.ErrUnreadable) { + t.Fatalf("a synced a broken providers.json: %v", err) + } + if fake.puts != puts { + t.Fatal("a pushed a broken providers.json") + } + if after, _ := os.ReadFile(provider.Path()); !bytes.Equal(after, broken) { + t.Fatal("a's broken providers.json was written over") + } + if _, err := backup.Collect(true, "test"); !errors.Is(err, provider.ErrUnreadable) { + t.Fatalf("a backup of a broken providers.json: %v", err) + } + remote, err := backup.Open(fake.files["/dav/magpie/magpie.magpie-backup"], "correct horse") + if err != nil || len(remote.Providers) != 1 { + t.Fatalf("on the server: %v %+v", err, remote.Providers) + } + b.use(t) + if err := Now(ctx); err != nil { + t.Fatal(err) + } + if got := ids(); !slices.Equal(got, []string{"deepseek=k1"}) { + t.Fatalf("b lost its providers to a's broken file: %v", got) + } +} diff --git a/internal/gui/assets/app.css b/internal/gui/assets/app.css index c7097462..ebca2a39 100644 --- a/internal/gui/assets/app.css +++ b/internal/gui/assets/app.css @@ -1572,6 +1572,9 @@ body.window .ag-sk-icon { width: 22px; height: 22px; border-radius: 6px; } #view-providers .after-list { position: sticky; bottom: -8px; z-index: 2; margin: 0 -12px -8px; padding: 14px 16px 12px; background: linear-gradient(to bottom, transparent, var(--bg) 12px); transition: opacity .18s ease, visibility .18s; } #view-providers .after-list.away { opacity: 0; visibility: hidden; } #movable { margin: 0 0 8px; } +#fileError { margin: 0 0 10px; } +#fileError .signing { border: 1px solid var(--line); border-radius: 10px; } +#fileError .signing .mark { background: var(--red); } .movable { display: flex; align-items: baseline; flex-wrap: wrap; gap: 0 4px; padding: 2px 14px; font-size: 11.5px; line-height: 1.6; color: var(--muted); } .movable .dot { width: 6px; height: 6px; border-radius: 50%; background: var(--accent); flex: none; margin-right: 4px; align-self: center; } .movable .link { font-size: 11.5px; padding: 0 2px; } diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index 05167533..efa306cc 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -2225,7 +2225,8 @@ async function loadProviders() { providers = await api("providers"); // a reload's provider may be gone since if (typeof editing === "string" && !providers.providers.some((p) => p.id === editing)) editing = null; - if (!providers.providers.length && editing === null) adding = true; + // a providers.json that can't be read is not a first use: no Add sheet + if (!providers.providers.length && editing === null && !providers.fileError) adding = true; if (view === "gateway") { // the gateway page is looked at and left open: coming back to the window // redraws it only when something on it changed, and when only the calls @@ -2350,6 +2351,7 @@ function renderProviders() { if (open) dialog = renderEditor(p); } renderExcluded(); + renderFileError(); renderMovable(); dialog = renderAdd() || dialog; if (importing) dialog = renderImport(importing); @@ -2390,6 +2392,26 @@ async function switchProvider(p, on, s) { } } +// renderFileError: over the list, that providers.json is there but can't +// be read, so the list is the signed-in accounts alone: magpie left the +// file as it is, and nothing is saved over it until it is fixed or moved. +function renderFileError() { + const box = $("#fileError"); + if (!box) return; + box.replaceChildren(); + box.hidden = !providers.fileError; + if (box.hidden) return; + const r = el("div", "signing file-error"); + r.setAttribute("role", "alert"); + r.append(el("span", "mark", "!")); + const tt = el("span", "tt"); + tt.append(el("span", "n", t("Your providers file can't be read")), + el("span", "s", t("magpie left it unchanged and lists no providers from it. Fix the file or move it aside, then reopen this page; until then, changes to providers are refused.")), + el("span", "s", providers.fileError)); + r.append(tt); + box.append(r); +} + // renderMovable: one quiet line over the list naming the built-in // subscriptions a community plugin can run, and Review, which opens the // first one's editor at its Runs on. Hidden, it stays hidden until another diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index d156246f..8bd412e9 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -321,6 +321,8 @@ const I18N = { "magpie forgets its copy of {users}. {agent}'s own files and sign-in, and the account itself, are left as they are.": "magpie 会移除它保存的 {users}。{agent} 自己的文件和登录，以及账号本身，都保持原样。", "signed in": "已登录", "{p} exposes no models yet — pick some below": "{p} 尚未暴露模型 — 在下方选几个", + "Your providers file can't be read": "无法读取供应商配置文件", + "magpie left it unchanged and lists no providers from it. Fix the file or move it aside, then reopen this page; until then, changes to providers are refused.": "magpie 没有改动它，也不会列出其中的供应商。请修复该文件或将其移走，然后重新打开此页；在此之前，对供应商的修改都会被拒绝。", "＋ Add provider": "＋ 添加供应商", "Add a provider": "添加供应商", "Add your first provider": "添加第一个供应商", diff --git a/internal/gui/assets/index.html b/internal/gui/assets/index.html index 7df95eb4..5ec394e3 100644 --- a/internal/gui/assets/index.html +++ b/internal/gui/assets/index.html @@ -73,6 +73,7 @@
+
diff --git a/internal/gui/providers.go b/internal/gui/providers.go index fb01b463..3019f26e 100644 --- a/internal/gui/providers.go +++ b/internal/gui/providers.go @@ -192,6 +192,10 @@ type providersJSON struct { // Moved is the agents the change moved off models it stopped serving // (agent.Reseat), for the page to say so. Moved []agent.Move `json:"moved,omitempty"` + // FileError is why providers.json can't be read (provider.FileError): + // the page says so over what is listed, which is then the signed-in + // accounts alone, never "add your first provider". + FileError string `json:"fileError,omitempty"` } // agentModel is the model an agent is on, as magpie's catalog names it. @@ -398,6 +402,9 @@ func providersState() providersJSON { agents := agent.Detected() s := providersJSON{Providers: []providerJSON{}, Presets: []presetJSON{}, Excluded: []excludedJSON{}} s.OnPlugins = provider.OnPlugins() + if err := provider.FileError(); err != nil { + s.FileError = err.Error() + } for _, x := range provider.Excluded() { e := excludedJSON{Exclusion: x, Name: x.Agent, Icon: "generic"} if a, err := agent.Find(x.Agent); err == nil { diff --git a/internal/gui/tests/README.md b/internal/gui/tests/README.md index 1ed6f172..ecd711bb 100644 --- a/internal/gui/tests/README.md +++ b/internal/gui/tests/README.md @@ -858,10 +858,18 @@ and what was typed kept, and Remove posts `{vendor, remove: true}`. No click moves the page. English and Chinese, Chromium and WebKit, with `/api/plugins` faked. +`provider-file-error.test.cjs` checks a providers.json that is there but +can't be read (#415's review): the Providers page says over the list that the +file can't be read (无法读取供应商配置文件), was left unchanged and is to be +fixed or moved aside, with magpie's reason; the Add sheet doesn't open as for +a first use, a signed-in account still listed stays under it, and a file that +reads shows none of it. English and Chinese, Chromium and WebKit, with +`/api/plugins` faked. + With Node.js and Playwright available: ```sh -node --test internal/gui/tests/menu-scroll.test.cjs internal/gui/tests/click-scroll.test.cjs internal/gui/tests/panel-fold.test.cjs internal/gui/tests/rollup-short.test.cjs internal/gui/tests/panel-routing.test.cjs internal/gui/tests/gateway-fold.test.cjs internal/gui/tests/routing-kind.test.cjs internal/gui/tests/balance-fix.test.cjs internal/gui/tests/cli-update.test.cjs internal/gui/tests/login-import.test.cjs internal/gui/tests/agy-launch.test.cjs internal/gui/tests/currency.test.cjs internal/gui/tests/usage-ledger.test.cjs internal/gui/tests/update-check.test.cjs internal/gui/tests/omarchy.test.cjs internal/gui/tests/brand.test.cjs internal/gui/tests/library-loading.test.cjs internal/gui/tests/provider-typed.test.cjs node --test internal/gui/tests/signin-callback.test.cjs internal/gui/tests/routing-steady.test.cjs internal/gui/tests/old-webkit.test.cjs internal/gui/tests/session-terminal.test.cjs internal/gui/tests/text-size.test.cjs internal/gui/tests/zcode-site.test.cjs internal/gui/tests/rail-tip.test.cjs internal/gui/tests/sync-update.test.cjs internal/gui/tests/plugin-signin.test.cjs internal/gui/tests/plugin-market.test.cjs internal/gui/tests/s3-sync.test.cjs internal/gui/tests/routing-wb-refused.test.cjs internal/gui/tests/routing-blocked.test.cjs internal/gui/tests/routing-shape.test.cjs internal/gui/tests/market-have.test.cjs internal/gui/tests/sessions-today.test.cjs internal/gui/tests/routing-side-calls.test.cjs internal/gui/tests/import-all-skills.test.cjs internal/gui/tests/routing-flood.test.cjs internal/gui/tests/routing-busy.test.cjs internal/gui/tests/own-skill.test.cjs internal/gui/tests/routing-manual.test.cjs internal/gui/tests/tray-usages.test.cjs internal/gui/tests/provider-remove.test.cjs internal/gui/tests/signin-link.test.cjs internal/gui/tests/tray-inuse.test.cjs internal/gui/tests/codex-auto-reset.test.cjs internal/gui/tests/routing-auto-reset.test.cjs internal/gui/tests/claude-usage-asked.test.cjs internal/gui/tests/claude-risk.test.cjs internal/gui/tests/library-toggle-status.test.cjs internal/gui/tests/library-all.test.cjs internal/gui/tests/routing-reroute-title.test.cjs internal/gui/tests/plain-names.test.cjs internal/gui/tests/panel-effort.test.cjs internal/gui/tests/agent-models-all-hidden.test.cjs internal/gui/tests/omp-roles.test.cjs internal/gui/tests/claude-effort.test.cjs internal/gui/tests/remote-magpie.test.cjs internal/gui/tests/qoder-cn-tile.test.cjs internal/gui/tests/provider-levels-scroll.test.cjs internal/gui/tests/signin-paste-codex.test.cjs internal/gui/tests/usage-alert.test.cjs internal/gui/tests/quota-families.test.cjs internal/gui/tests/provider-search.test.cjs internal/gui/tests/webdav-settings-refresh.test.cjs internal/gui/tests/lan-docker.test.cjs internal/gui/tests/plugin-key-hint.test.cjs internal/gui/tests/update-hide.test.cjs internal/gui/tests/tray-logos.test.cjs internal/gui/tests/routing-proxy.test.cjs internal/gui/tests/sse-body.test.cjs internal/gui/tests/remote-group-served.test.cjs internal/gui/tests/plugin-move-overflow.test.cjs internal/gui/tests/routing-time.test.cjs internal/gui/tests/unlisted-models.test.cjs internal/gui/tests/plugin-updates.test.cjs internal/gui/tests/request-archive.test.cjs internal/gui/tests/group-member-fast.test.cjs internal/gui/tests/providers-off-fold.test.cjs internal/gui/tests/account-return.test.cjs internal/gui/tests/usage-ledger-detail.test.cjs internal/gui/tests/usage-ledger-chart.test.cjs internal/gui/tests/panel-usage.test.cjs internal/gui/tests/usage-refresh.test.cjs internal/gui/tests/usage-ledger-content.test.cjs internal/gui/tests/claude-direct.test.cjs internal/gui/tests/signin-again.test.cjs internal/gui/tests/sessions-calendar-fill.test.cjs internal/gui/tests/editor-short-window.test.cjs internal/gui/tests/group-from-model-add.test.cjs internal/gui/tests/sync-other-kind.test.cjs internal/gui/tests/web-page.test.cjs internal/gui/tests/balance-parts.test.cjs internal/gui/tests/search-api.test.cjs +node --test internal/gui/tests/menu-scroll.test.cjs internal/gui/tests/click-scroll.test.cjs internal/gui/tests/panel-fold.test.cjs internal/gui/tests/rollup-short.test.cjs internal/gui/tests/panel-routing.test.cjs internal/gui/tests/gateway-fold.test.cjs internal/gui/tests/routing-kind.test.cjs internal/gui/tests/balance-fix.test.cjs internal/gui/tests/cli-update.test.cjs internal/gui/tests/login-import.test.cjs internal/gui/tests/agy-launch.test.cjs internal/gui/tests/currency.test.cjs internal/gui/tests/usage-ledger.test.cjs internal/gui/tests/update-check.test.cjs internal/gui/tests/omarchy.test.cjs internal/gui/tests/brand.test.cjs internal/gui/tests/library-loading.test.cjs internal/gui/tests/provider-typed.test.cjs node --test internal/gui/tests/signin-callback.test.cjs internal/gui/tests/routing-steady.test.cjs internal/gui/tests/old-webkit.test.cjs internal/gui/tests/session-terminal.test.cjs internal/gui/tests/text-size.test.cjs internal/gui/tests/zcode-site.test.cjs internal/gui/tests/rail-tip.test.cjs internal/gui/tests/sync-update.test.cjs internal/gui/tests/plugin-signin.test.cjs internal/gui/tests/plugin-market.test.cjs internal/gui/tests/s3-sync.test.cjs internal/gui/tests/routing-wb-refused.test.cjs internal/gui/tests/routing-blocked.test.cjs internal/gui/tests/routing-shape.test.cjs internal/gui/tests/market-have.test.cjs internal/gui/tests/sessions-today.test.cjs internal/gui/tests/routing-side-calls.test.cjs internal/gui/tests/import-all-skills.test.cjs internal/gui/tests/routing-flood.test.cjs internal/gui/tests/routing-busy.test.cjs internal/gui/tests/own-skill.test.cjs internal/gui/tests/routing-manual.test.cjs internal/gui/tests/tray-usages.test.cjs internal/gui/tests/provider-remove.test.cjs internal/gui/tests/signin-link.test.cjs internal/gui/tests/tray-inuse.test.cjs internal/gui/tests/codex-auto-reset.test.cjs internal/gui/tests/routing-auto-reset.test.cjs internal/gui/tests/claude-usage-asked.test.cjs internal/gui/tests/claude-risk.test.cjs internal/gui/tests/library-toggle-status.test.cjs internal/gui/tests/library-all.test.cjs internal/gui/tests/routing-reroute-title.test.cjs internal/gui/tests/plain-names.test.cjs internal/gui/tests/panel-effort.test.cjs internal/gui/tests/agent-models-all-hidden.test.cjs internal/gui/tests/omp-roles.test.cjs internal/gui/tests/claude-effort.test.cjs internal/gui/tests/remote-magpie.test.cjs internal/gui/tests/qoder-cn-tile.test.cjs internal/gui/tests/provider-levels-scroll.test.cjs internal/gui/tests/signin-paste-codex.test.cjs internal/gui/tests/usage-alert.test.cjs internal/gui/tests/quota-families.test.cjs internal/gui/tests/provider-search.test.cjs internal/gui/tests/webdav-settings-refresh.test.cjs internal/gui/tests/lan-docker.test.cjs internal/gui/tests/plugin-key-hint.test.cjs internal/gui/tests/update-hide.test.cjs internal/gui/tests/tray-logos.test.cjs internal/gui/tests/routing-proxy.test.cjs internal/gui/tests/sse-body.test.cjs internal/gui/tests/remote-group-served.test.cjs internal/gui/tests/plugin-move-overflow.test.cjs internal/gui/tests/routing-time.test.cjs internal/gui/tests/unlisted-models.test.cjs internal/gui/tests/plugin-updates.test.cjs internal/gui/tests/request-archive.test.cjs internal/gui/tests/group-member-fast.test.cjs internal/gui/tests/providers-off-fold.test.cjs internal/gui/tests/account-return.test.cjs internal/gui/tests/usage-ledger-detail.test.cjs internal/gui/tests/usage-ledger-chart.test.cjs internal/gui/tests/panel-usage.test.cjs internal/gui/tests/usage-refresh.test.cjs internal/gui/tests/usage-ledger-content.test.cjs internal/gui/tests/claude-direct.test.cjs internal/gui/tests/signin-again.test.cjs internal/gui/tests/sessions-calendar-fill.test.cjs internal/gui/tests/editor-short-window.test.cjs internal/gui/tests/group-from-model-add.test.cjs internal/gui/tests/sync-other-kind.test.cjs internal/gui/tests/web-page.test.cjs internal/gui/tests/balance-parts.test.cjs internal/gui/tests/search-api.test.cjs internal/gui/tests/provider-file-error.test.cjs ``` If Playwright is installed outside the repository, set `NODE_PATH` to the diff --git a/internal/gui/tests/provider-file-error.test.cjs b/internal/gui/tests/provider-file-error.test.cjs new file mode 100644 index 00000000..712c0cae --- /dev/null +++ b/internal/gui/tests/provider-file-error.test.cjs @@ -0,0 +1,94 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// A providers.json that is there but can't be read (#415's review) is not +// an empty list: the Providers page says over the list that the file can't +// be read, was left unchanged, and is to be fixed or moved aside, with the +// reason magpie gives; the Add sheet doesn't open as for a first use, and +// the signed-in accounts still listed stay. A file that reads shows none of +// it. In English and Chinese, Chromium and WebKit. +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 REASON = "/home/me/.config/magpie/providers.json can't be read (unexpected end of JSON input); magpie left it unchanged — fix it or move it aside"; + +function serve(lang, list, fileError) { + const providers = () => ({ providers: list, presets: [], excluded: [], gateway: { running: true, window: true }, ...(fileError ? { fileError } : {}) }); + const state = { agents: [], profiles: [], settings: { lang, theme: "light" } }; + return async (route) => { + const url = new URL(route.request().url()); + const json = (data) => route.fulfill({ 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); + if (url.pathname === "/api/providers") return json(providers()); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname === "/api/groups") return json({ groups: [] }); + 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)]; + try { await route.fulfill({ body: await fs.readFile(file), contentType }); } catch { await route.fulfill({ status: 404, body: "" }); } + }; +} + +const account = { + id: "codex", name: "Codex", icon: "codex-color", chat: "", responses: "https://chatgpt.com/backend-api/codex", anthropic: "", catalog: "", + host: "chatgpt.com", models: [{ id: "gpt-5", name: "GPT-5", on: true }], agents: [], fallback: [], headers: {}, + key: { set: false }, keyList: [], balanceToken: { takes: false, set: false }, proxy: "", ready: true, + account: { agent: "codex", agentName: "Codex", user: "me@example.com", plan: "Plus", logins: [] }, +}; +const heading = { en: "Your providers file can't be read", zh: "无法读取供应商配置文件" }; +const body = { en: "Fix the file or move it aside", zh: "请修复该文件或将其移走" }; + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + for (const lang of ["en", "zh"]) { + test(`${engine} ${lang}: an unreadable providers.json is said, not shown as no providers`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + const page = await (await browser.newContext({ viewport: { width: 900, height: 560 }, reducedMotion: "reduce" })).newPage(); + t.after(async () => { + if (process.env.ARTIFACT_DIR) { + await fs.mkdir(process.env.ARTIFACT_DIR, { recursive: true }); + await page.screenshot({ path: path.join(process.env.ARTIFACT_DIR, `${engine}-${lang}-provider-file-error.png`) }); + } + await browser.close(); + }); + page.setDefaultTimeout(5000); + const errors = []; + page.on("pageerror", (e) => errors.push(e.message)); + + // nothing listed: the error, and no Add sheet as for a first use + await page.route("**/*", serve(lang, [], REASON)); + await page.goto("http://magpie.test/?view=providers"); + const box = page.locator("#fileError"); + await box.locator(".signing").waitFor(); + assert(await box.isVisible(), "the error shows"); + const text = await box.textContent(); + assert(text.includes(heading[lang]), text); + assert(text.includes(body[lang]), text); + assert(text.includes(REASON), "the reason is said"); + assert.equal(await box.locator('[role="alert"]').count(), 1); + await page.waitForTimeout(200); + assert(await page.locator("#addSheet").evaluate((s) => s.hidden), "the Add sheet opened as for a first use"); + + // a signed-in account still listed stays, under the error + await page.unroute("**/*"); + await page.route("**/*", serve(lang, [account], REASON)); + await page.reload(); + await page.locator('#providers .row.provider[data-id="codex"]').waitFor(); + assert(await box.isVisible(), "the error shows over the accounts"); + const errBottom = await box.evaluate((e) => e.getBoundingClientRect().bottom); + const listTop = await page.locator("#providers").evaluate((e) => e.getBoundingClientRect().top); + assert(errBottom <= listTop, "the error is over the list"); + + // a file that reads: none of it + await page.unroute("**/*"); + await page.route("**/*", serve(lang, [account], "")); + await page.reload(); + await page.locator('#providers .row.provider[data-id="codex"]').waitFor(); + assert(await box.evaluate((b) => b.hidden), "the error shows for a file that reads"); + assert.deepEqual(errors, []); + }); + } +} diff --git a/internal/provider/backup.go b/internal/provider/backup.go index ec71f64e..f2396677 100644 --- a/internal/provider/backup.go +++ b/internal/provider/backup.go @@ -6,8 +6,13 @@ package provider import "slices" // Stored is the providers as providers.json keeps them: keys included, -// signed-in accounts only as the model picks the user made for them. -func Stored() []Provider { return load().Providers } +// signed-in accounts only as the model picks the user made for them. A +// file that can't be read is an error, never none: a backup or a sync +// carrying none would take every provider away where it is put back. +func Stored() ([]Provider, error) { + f, err := read() + return f.Providers, err +} // Restore puts providers from a backup in. Each replaces the one here with // its id; one that came without keys keeps the keys already here. It @@ -50,8 +55,11 @@ func Restore(ps []Provider) (added, replaced int, err error) { } // StoredGroups is the groups as saved: the user's own, and the found ones -// the user removed. -func StoredGroups() []Group { return load().Groups } +// the user removed. As with Stored, a file that can't be read is an error. +func StoredGroups() ([]Group, error) { + f, err := read() + return f.Groups, err +} // RestoreGroups puts groups from a backup in, each replacing the one here // with its id. diff --git a/internal/provider/backup_test.go b/internal/provider/backup_test.go index c50ff01d..401ce09d 100644 --- a/internal/provider/backup_test.go +++ b/internal/provider/backup_test.go @@ -48,7 +48,7 @@ func TestBackupBalanceToken(t *testing.T) { if err := op.put([]Provider{p}); err != nil { t.Fatal(err) } - ps := Stored() + ps, _ := Stored() if len(ps) != 1 { t.Fatalf("providers: %+v", ps) } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index af3d650e..e48460a1 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -23,6 +23,7 @@ import ( "github.com/yetone/magpie/internal/catalog" "github.com/yetone/magpie/internal/plugin" "github.com/yetone/magpie/internal/settings" + "github.com/yetone/magpie/internal/steady" ) // Protocol is a wire API magpie can speak to an upstream. @@ -197,28 +198,57 @@ func Path() string { return filepath.Join(home, ".config", "magpie", "providers.json") } +// load is the file for a read that goes on without it: start-up, the +// gateway, the catalog. One that can't be read is taken as empty there; +// what lists the providers to the user says so (FileError), and an edit +// refuses (read). func load() file { f, _ := read() return f } +// ErrUnreadable is a providers.json that is there but can't be read or +// decoded: never an empty catalog, to list as none, back up or write over. +var ErrUnreadable = errors.New("providers.json can't be read") + +type unreadableError struct { + path string + err error +} + +func (e *unreadableError) Error() string { + return fmt.Sprintf("%s can't be read (%v); magpie left it unchanged — fix it or move it aside", e.path, e.err) +} + +func (e *unreadableError) Unwrap() []error { return []error{ErrUnreadable, e.err} } + // Reads used for an edit must keep errors: a broken file is not an empty // catalog to write over. Only a missing file is a first use. func read() (file, error) { var f file - b, err := os.ReadFile(Path()) + b, err := steady.ReadFile(Path()) if errors.Is(err, os.ErrNotExist) { return f, nil } if err != nil { - return f, err + return f, &unreadableError{Path(), err} } if err := json.Unmarshal(b, &f); err != nil { - return file{}, fmt.Errorf("%s: %w", Path(), err) + return file{}, &unreadableError{Path(), err} } return f, nil } +// FileError is why providers.json can't be read, nil when it can or isn't +// there: for what lists the providers to say so, not "none yet". +func FileError() error { + _, err := read() + return err +} + +// store replaces providers.json whole, through a file renamed over it, so +// that a read at that moment sees the old catalog or the new one, never a +// file cut short. func store(f file) error { p := Path() if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { @@ -228,13 +258,10 @@ func store(f file) error { if err != nil { return err } - if err := os.WriteFile(p, append(b, '\n'), 0o600); err != nil { + if err := writePrivate(p, append(b, '\n')); err != nil { return err } pruneIcons(f) - if err := os.Chmod(p, 0o600); err != nil { - return err - } // what agents were handed of the catalog may be out of date now catalog.Touched() return nil diff --git a/internal/provider/store_unreadable_test.go b/internal/provider/store_unreadable_test.go new file mode 100644 index 00000000..fc3f8cf8 --- /dev/null +++ b/internal/provider/store_unreadable_test.go @@ -0,0 +1,125 @@ +package provider + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func writeProviderFile(t *testing.T, data string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(Path()), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(Path(), []byte(data), 0o600); err != nil { + t.Fatal(err) + } +} + +// What lists the providers learns that providers.json can't be read +// (FileError), rather than taking it for none; start-up and the gateway +// still go on with it as empty (#415's review). +func TestFileErrorSaysUnreadable(t *testing.T) { + providerFileHome(t) + if err := FileError(); err != nil { + t.Fatalf("no file is a first use: %v", err) + } + writeProviderFile(t, originalProviderFile) + if err := FileError(); err != nil { + t.Fatalf("a file that reads: %v", err) + } + writeProviderFile(t, originalProviderFile[:40]) + err := FileError() + if !errors.Is(err, ErrUnreadable) { + t.Fatalf("a file cut short: %v", err) + } + for _, want := range []string{Path(), "can't be read", "left it unchanged", "move it aside"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("%q doesn't say %q", err, want) + } + } + if ps := load().Providers; len(ps) != 0 { + t.Fatalf("load of a broken file: %+v", ps) + } + _ = All() + _ = Catalog() +} + +// An edit refused over a broken file says it was left unchanged and is to +// be fixed or moved aside, not only the decoder's complaint. +func TestWriteErrorsSayLeftUnchanged(t *testing.T) { + p := Provider{ID: "new", Name: "New", Chat: "https://new.example.invalid/v1", Key: "synthetic-new"} + for name, run := range map[string]func() error{ + "save": func() error { return Save(p) }, + "restore": func() error { _, _, err := Restore([]Provider{p}); return err }, + "restore-groups": func() error { return RestoreGroups([]Group{{ID: "g", Members: []string{"new/m"}}}) }, + "mirror": func() error { return Mirror([]Provider{p}, nil) }, + } { + t.Run(name, func(t *testing.T) { + providerFileHome(t) + writeProviderFile(t, "{") + err := run() + if !errors.Is(err, ErrUnreadable) || !strings.Contains(err.Error(), "left it unchanged") || !strings.Contains(err.Error(), "fix it or move it aside") { + t.Fatalf("got %v", err) + } + }) + } +} + +// Stored and StoredGroups, which backups and sync carry, never give a +// broken file as no providers: another computer mirroring that would lose +// all of its own. +func TestStoredRefusesUnreadable(t *testing.T) { + providerFileHome(t) + if ps, err := Stored(); err != nil || len(ps) != 0 { + t.Fatalf("no file: %v %v", ps, err) + } + writeProviderFile(t, originalProviderFile) + if ps, err := Stored(); err != nil || len(ps) != 1 { + t.Fatalf("a file that reads: %v %v", ps, err) + } + if gs, err := StoredGroups(); err != nil || len(gs) != 1 { + t.Fatalf("a file that reads: %v %v", gs, err) + } + writeProviderFile(t, originalProviderFile[:len(originalProviderFile)-2]) + if _, err := Stored(); !errors.Is(err, ErrUnreadable) { + t.Fatalf("Stored of a broken file: %v", err) + } + if _, err := StoredGroups(); !errors.Is(err, ErrUnreadable) { + t.Fatalf("StoredGroups of a broken file: %v", err) + } +} + +// providers.json is replaced through a file renamed over it, never cut +// short and written again in place: a read at that moment sees one whole +// catalog or the other. The file stays the user's alone. +func TestStoreReplacesAtomically(t *testing.T) { + providerFileHome(t) + writeProviderFile(t, originalProviderFile) + before, err := os.Stat(Path()) + if err != nil { + t.Fatal(err) + } + if err := Save(Provider{ID: "new", Name: "New", Chat: "https://new.example.invalid/v1", Key: "synthetic-new"}); err != nil { + t.Fatal(err) + } + after, err := os.Stat(Path()) + if err != nil { + t.Fatal(err) + } + if os.SameFile(before, after) { + t.Fatal("providers.json was written in place, not renamed over") + } + if runtime.GOOS != "windows" && after.Mode().Perm() != 0o600 { + t.Fatalf("providers.json is %v", after.Mode().Perm()) + } + if ps, err := Stored(); err != nil || len(ps) != 2 { + t.Fatalf("after the save: %v %v", ps, err) + } + if left, _ := filepath.Glob(filepath.Join(filepath.Dir(Path()), ".providers.json.*")); len(left) != 0 { + t.Fatalf("temp files left: %v", left) + } +} diff --git a/internal/tui/pages.go b/internal/tui/pages.go index 2e25f1ab..6bf5fed4 100644 --- a/internal/tui/pages.go +++ b/internal/tui/pages.go @@ -57,6 +57,7 @@ type balanceMsg map[string]string func (m *model) reloadProviders() { m.provs = provider.All() + m.provsErr = provider.FileError() m.prow = clamp(m.prow, len(m.provs)) } @@ -377,8 +378,13 @@ func (m model) viewProviders() string { var b strings.Builder b.WriteString(m.header()) b.WriteString("\n\n") + if m.provsErr != nil { + b.WriteString(pad + " " + sBad.Render("! "+m.provsErr.Error()) + "\n\n") + } if len(m.provs) == 0 { - b.WriteString(pad + " " + sMuted.Render("no providers yet · a adds one")) + if m.provsErr == nil { + b.WriteString(pad + " " + sMuted.Render("no providers yet · a adds one")) + } return b.String() } type row struct{ name, id, key, models, note string } diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 12d09c59..1debbcb6 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -102,6 +102,7 @@ type model struct { ask ask confirm string // what a second d removes provs []provider.Provider + provsErr error // providers.json can't be read: said, not "none yet" prow int bal map[string]string asked bool // balances were asked for diff --git a/providers_cli.go b/providers_cli.go index 7ffa9317..ee8abd91 100644 --- a/providers_cli.go +++ b/providers_cli.go @@ -68,7 +68,13 @@ const providerUsage = `usage: // providers: `magpie providers` func providers() error { all := provider.All() + // a providers.json that can't be read is not "no providers yet": what + // is listed is then the signed-in accounts alone, and it ends in why + bad := provider.FileError() if len(all) == 0 && len(provider.Excluded()) == 0 { + if bad != nil { + return bad + } fmt.Println(muted.Render("no providers yet ·"), "magpie provider add deepseek sk-…", muted.Render("· magpie presets lists the vendors")) return nil } @@ -129,7 +135,7 @@ func providers() error { } fmt.Println(" ", muted.Render(name+" is signed in but not offered: "+x.Why+back)) } - return nil + return bad } // usesByProvider maps provider ids to the agents currently routed to them. @@ -183,6 +189,7 @@ func presets() error { // one agent is shown and what is kept from it func models(args []string) error { entries := provider.Catalog() + bad := provider.FileError() // as in providers: said, not "no models yet" var hidden []provider.Entry agentID := "" if len(args) > 0 { @@ -198,6 +205,8 @@ func models(args []string) error { if len(entries) == 0 && agentID != "" { names, _ := provider.VisibleTo(agentID) fmt.Println(amber.Render("!"), agentID, "is shown none of them: nothing is in", strings.Join(names, ", "), muted.Render("· magpie visible "+agentID+" all shows it every model")) + } else if len(entries) == 0 && bad != nil { + return bad } else if len(entries) == 0 { fmt.Println(muted.Render("no models yet · add a provider first:"), "magpie provider add deepseek sk-…") return nil @@ -229,7 +238,7 @@ func models(args []string) error { explainHidden(agentID, hidden) } fmt.Println(faint.Render(" " + gateway.URL() + "/v1")) - return nil + return bad } // providerCmd: `magpie provider …` @@ -827,6 +836,9 @@ func serve() error { fmt.Println(l) } n := len(provider.Catalog()) + if err := provider.FileError(); err != nil { + fmt.Println(amber.Render("!"), err) // served without it, as no providers + } if n == 0 { fmt.Println(amber.Render("!"), "no models yet ·", "magpie provider add deepseek sk-…") } else { diff --git a/providers_unreadable_cli_test.go b/providers_unreadable_cli_test.go new file mode 100644 index 00000000..460a42bc --- /dev/null +++ b/providers_unreadable_cli_test.go @@ -0,0 +1,68 @@ +package main + +import ( + "bytes" + "errors" + "io" + "os" + "strings" + "testing" + + "github.com/yetone/magpie/internal/catalog" + "github.com/yetone/magpie/internal/provider" +) + +// printed is what fn printed, and the error it ended with. +func printed(t *testing.T, fn func() error) (string, error) { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + old := os.Stdout + os.Stdout = w + var out strings.Builder + done := make(chan struct{}) + go func() { + b, _ := io.ReadAll(r) + out.Write(b) + close(done) + }() + err = fn() + w.Close() + os.Stdout = old + <-done + r.Close() + return out.String(), err +} + +// A providers.json that can't be read is not "no providers yet": magpie +// providers and magpie models end in why, and leave the file as it is +// (#415's review). +func TestProvidersCmdSaysUnreadable(t *testing.T) { + groupsHome(t) + catalog.Changed = nil + whole, err := os.ReadFile(provider.Path()) + if err != nil { + t.Fatal(err) + } + broken := whole[:len(whole)/2] + if err := os.WriteFile(provider.Path(), broken, 0o600); err != nil { + t.Fatal(err) + } + for name, fn := range map[string]func() error{ + "providers": providers, + "models": func() error { return models(nil) }, + } { + out, err := printed(t, fn) + if !errors.Is(err, provider.ErrUnreadable) || !strings.Contains(err.Error(), "left it unchanged") { + t.Errorf("%s: %v", name, err) + } + if strings.Contains(out, "yet") { + t.Errorf("%s said there are none yet: %q", name, out) + } + } + if after, _ := os.ReadFile(provider.Path()); !bytes.Equal(after, broken) { + t.Fatal("the broken providers.json was changed") + } +}