diff --git a/internal/plugin/host.js b/internal/plugin/host.js index 3ceead3a..3c466472 100644 --- a/internal/plugin/host.js +++ b/internal/plugin/host.js @@ -569,6 +569,9 @@ async function info(id, key, strict) { if (strict && !(e?.signIn === "expired" && all[k])) throw e const r = reach.getStore() if (r && e?.signIn === "expired") r.refused ??= e?.message ?? String(e) + // a hook that threw on its vendor's failure has no list to tell: + // magpie keeps the one it had, as for one that gave its defaults back + out.fellBack = true send({ event: "log", level: "error", message: `${h.spec}: provider.models: ${e?.message ?? e}` }) } } @@ -615,8 +618,9 @@ async function providers() { const p = await info(id) // each account's own models, as the built-ins read each account's: a // plan may serve fewer, or others, than the first account's. One that - // can't be read is taken to have them all. - const own = await Promise.all(keys.slice(1).map((k) => info(id, k, true).catch(() => null))) + // can't be read is taken to have them all, or the ones it was last + // told to have, as a built-in account whose fetch failed keeps its own. + const own = await Promise.all(keys.slice(1).map((k) => info(id, k, true).catch(() => ({ failed: true })))) const models = { ...p.models } for (const q of own) for (const [k, m] of Object.entries(q?.models ?? {})) models[k] ??= m const ids = (q) => Object.values(q.models).filter((m) => m.status !== "deprecated").map((m) => m.id) @@ -639,8 +643,8 @@ async function providers() { type: stored[k]?.type ?? "", accountId: whoOf(stored[k]), hint: hintOf(stored[k]), - models: own.length === 0 ? undefined : i === 0 ? ids(p) : own[i - 1] ? ids(own[i - 1]) : undefined, - fellBack: i === 0 ? !!p.fellBack : !!own[i - 1]?.fellBack, + models: own.length === 0 ? undefined : i === 0 ? ids(p) : own[i - 1]?.models ? ids(own[i - 1]) : undefined, + fellBack: i === 0 ? !!p.fellBack : !!(own[i - 1]?.fellBack || own[i - 1]?.failed), })), models: Object.values(models) .filter((m) => m.status !== "deprecated") diff --git a/internal/plugin/plugin_listed_test.go b/internal/plugin/plugin_listed_test.go index b011c57c..a3f89ad5 100644 --- a/internal/plugin/plugin_listed_test.go +++ b/internal/plugin/plugin_listed_test.go @@ -15,9 +15,17 @@ import ( // A plugin whose models hook can't reach its vendor and gives its // defaults back keeps the list it told last, for the provider and each // account, as a built-in whose fetch fails keeps the one it fetched last; -// the vendor back, its list is told again. +// the vendor back, its list is told again. So too for a hook that throws +// (Cursor's, Grok's, Devin's) rather than give its defaults back. func TestFallenBackListKeepsTheLastOne(t *testing.T) { + for _, throws := range []string{"", "1"} { + t.Run("throws="+throws, func(t *testing.T) { fallenBack(t, throws) }) + } +} + +func fallenBack(t *testing.T, throws string) { sandbox(t) + t.Setenv("FAKE_MODELS_THROW", throws) var down atomic.Bool var name atomic.Value name.Store("extra") diff --git a/internal/plugin/testdata/fake/index.js b/internal/plugin/testdata/fake/index.js index a21556ff..cde0e43d 100644 --- a/internal/plugin/testdata/fake/index.js +++ b/internal/plugin/testdata/fake/index.js @@ -113,7 +113,11 @@ export const FakePlugin = async ({ client }) => ({ if (auth?.key === "few") return { "fake-1": p.models["fake-1"] } // $FAKE_MODELS: the vendor's list, whose answer names one more model if (process.env.FAKE_MODELS && auth) { - const r = await fetch(process.env.FAKE_MODELS).then((r) => r.text()).catch(() => "") + const r = await fetch(process.env.FAKE_MODELS).then((r) => r.text()).catch((e) => { + // $FAKE_MODELS_THROW: the hook throws, as Cursor's, Grok's and Devin's do + if (process.env.FAKE_MODELS_THROW) throw e + return "" + }) if (r) p.models["fake-" + r] = { ...p.models["fake-1"], id: "fake-" + r, name: r } } return p.models