From b0f4a2dba385ad70fa4d6c37922ccd9bdee0e1c5 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Tue, 1 Sep 2026 10:39:19 +0800 Subject: [PATCH] fix: resolve the qwen model catalog from the gateway, not qwen-code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model picker rendered empty for the qwen backend against a gateway serving a dozen models. qwen-code's `getAvailableModels()` is not a catalog fetch. It returns `modelRegistry.getModelsForAuthType(currentAuthType)` — an in-memory registry seeded once at construction from the hardcoded QWEN_OAUTH_MODELS (a single entry, `coder-model`) for qwen-oauth, and from the user's `modelProviders` setting for every other authType. Under `authType: "openai"` with no `modelProviders` declared it returns `[]`, verified against the live SDK. qwen-code never asks the gateway what it hosts, so there was nothing for codeoid to cache — `_cacheModels` correctly ignores empty reports. Ask the gateway directly instead. On the openai path it is an OpenAI-compatible endpoint, so `GET /models` is authoritative and stays current as the provider adds models — no hand-maintained list on any box. The qwen-code registry is unioned in rather than replaced: it carries real display labels, and qwen-oauth has a dynamic base URL and no key to present, so HTTP is not an option there. A failed fetch falls back to the registry; the session still runs, only the picker is poorer. Everything the endpoint reports is surfaced. The response carries only {id, object, created, owned_by} with no modality field, so filtering out image/audio entries would mean pattern-matching ids — the same hardcoded-list problem this removes. Two supporting fixes: - normalizeModelCatalog read `name` for the display label, but the SDK emits `label` (its handler projects the registry entry down to {id, label, capabilities, contextWindowSize}). Every model rendered as its raw id. `name`/`modelId`/`availableModels` stay as accepted aliases. - _cacheModels was first-non-empty-wins per daemon lifetime. Providers report on each query-loop build, so a model added to a gateway now appears on the next session instead of after a daemon restart. Empty reports are still ignored so a failed fetch cannot clobber a good catalog, and an unchanged report skips the SQLite write. Co-Authored-By: Claude Opus 5 (1M context) --- src/daemon/providers/qwen/index.ts | 157 ++++++++++++++++++++++++++--- src/daemon/session-manager.ts | 28 +++-- src/tests/models.test.ts | 14 ++- src/tests/provider-qwen.test.ts | 105 +++++++++++++++++++ 4 files changed, 281 insertions(+), 23 deletions(-) diff --git a/src/daemon/providers/qwen/index.ts b/src/daemon/providers/qwen/index.ts index 2bdcd0f..d69ed4d 100644 --- a/src/daemon/providers/qwen/index.ts +++ b/src/daemon/providers/qwen/index.ts @@ -114,6 +114,9 @@ export class QwenProvider implements SessionProvider { #seenSubagents = new Set(); /** Model the live loop was built with — the turn_done attribution fallback. */ #currentModel: string | null = null; + /** Resolved gateway URL + credential path of the live loop — see #loadCatalog. */ + #currentBaseUrl: string | null = null; + #currentAuthType: "openai" | "qwen-oauth" | null = null; constructor(init: QwenProviderInit) { this.#backingId = coerceBackingId(init.initialBackingId, init.sessionId); @@ -198,12 +201,59 @@ export class QwenProvider implements SessionProvider { } async listModels(): Promise { - if (!this.#query) return []; + return this.#loadCatalog(); + } + + /** + * The model catalog, unioned from both sources it can come from. + * + * qwen-code's `getAvailableModels()` is NOT a catalog fetch — it returns + * `modelRegistry.getModelsForAuthType(currentAuthType)`, an in-memory + * registry seeded once at construction from (a) the hardcoded + * `QWEN_OAUTH_MODELS` (a single entry, `coder-model`) for `qwen-oauth` and + * (b) the user's `modelProviders` setting for every other authType. Under + * `authType: "openai"` with no `modelProviders` declared it returns `[]`, + * which is why the picker used to render empty against a gateway serving a + * dozen models: qwen-code never asks the gateway what it hosts. + * + * So on the `openai` path we ask the gateway ourselves — it is an + * OpenAI-compatible endpoint, so `GET /models` is authoritative and stays + * current as the provider adds models. The registry is still unioned in + * (rather than replaced) because it carries real display labels, and because + * `qwen-oauth` has a dynamic base URL and no key to present, so HTTP is not + * an option there and the registry is the only source. + * + * Everything the endpoint reports is returned. The response carries only + * `{id, object, created, owned_by}` — no modality field — so filtering + * non-chat entries (image/audio models) would mean pattern-matching ids, + * which is exactly the kind of hardcoded list this method exists to remove. + */ + async #loadCatalog(): Promise { + const registry = this.#query + ? await this.#query + .getAvailableModels() + .then(normalizeModelCatalog) + .catch(() => [] as ModelInfo[]) + : []; + + if (this.#currentAuthType !== "openai") return registry; + + const baseUrl = this.#currentBaseUrl ?? process.env.OPENAI_BASE_URL; + const apiKey = process.env.OPENAI_API_KEY; + if (!baseUrl || !apiKey) return registry; + try { - const raw = await this.#query.getAvailableModels(); - return normalizeModelCatalog(raw); - } catch { - return []; + const live = await fetchOpenAiModelCatalog(baseUrl, apiKey); + return unionCatalogs(live, registry); + } catch (err) { + // A catalog fetch is best-effort: the session still runs on the + // configured model, only the picker is poorer for it. + console.error( + `[qwen-provider ${this.#init.sessionId.slice(0, 8)}] model catalog fetch failed (${ + err instanceof Error ? err.message : String(err) + }) — falling back to the qwen-code registry`, + ); + return registry; } } @@ -313,6 +363,8 @@ export class QwenProvider implements SessionProvider { const baseUrl = resolveQwenBaseUrl(qwenCfg?.baseUrl); const model = opts.model ?? qwenCfg?.model; this.#currentModel = model ?? null; + this.#currentBaseUrl = baseUrl ?? null; + this.#currentAuthType = authType; this.#query = query({ prompt: this.#inputQueue, @@ -385,10 +437,10 @@ export class QwenProvider implements SessionProvider { this.#hasQueried = true; if (init.onModels) { - void this.#query - .getAvailableModels() - .then((raw) => { - const models = normalizeModelCatalog(raw); + // Fired on every loop build, so a model added to the gateway shows up on + // the next session rather than waiting for a daemon restart. + void this.#loadCatalog() + .then((models) => { if (models.length > 0) { init.onModels?.( models.map((m) => ({ @@ -752,9 +804,17 @@ export function resolveQwenAuthType( /** * Normalize `Query.getAvailableModels()`, which is typed only as - * `Record | null`. Accepts the observed - * `{ availableModels: [{ modelId, name, description }] }` shape plus a bare - * array, and ignores anything else rather than throwing. + * `Record | null`. + * + * The shape actually emitted by @qwen-code/sdk 0.1.8 is + * `{ subtype, models: [{ id, label, capabilities, contextWindowSize }] }` — + * note `label`, not `name`, and no `description` (the CLI's + * `handleGetAvailableModels` projects the registry entry down to those four + * fields, dropping the description the registry itself carries). `name` / + * `modelId` / `availableModels` are kept as accepted aliases so a future + * rename doesn't silently empty the picker. + * + * A bare array is accepted too; anything else yields `[]` rather than throwing. */ export function normalizeModelCatalog(raw: unknown): ModelInfo[] { const list = Array.isArray(raw) @@ -774,15 +834,86 @@ export function normalizeModelCatalog(raw: unknown): ModelInfo[] { const e = entry as Record; const id = typeof e.modelId === "string" ? e.modelId : typeof e.id === "string" ? e.id : null; if (!id) continue; + const label = + typeof e.label === "string" ? e.label : typeof e.name === "string" ? e.name : id; out.push({ id, - displayName: typeof e.name === "string" ? e.name : id, + displayName: label, ...(typeof e.description === "string" ? { description: e.description } : {}), }); } return out; } +/** Give up on a catalog fetch well inside any reasonable session-start budget. */ +const CATALOG_FETCH_TIMEOUT_MS = 10_000; + +/** + * Fetch an OpenAI-compatible `GET /models` catalog. + * + * Every gateway codeoid points the qwen backend at (DashScope, the Bailian + * token-plan host, or a bring-your-own URL) speaks the OpenAI wire protocol — + * that is the whole premise of `authType: "openai"` — so `/models` is + * available and authoritative. Response shape is + * `{ object: "list", data: [{ id, object, created, owned_by }] }`; only `id` + * is load-bearing here. + * + * Exported for unit testing. Throws on transport error or non-2xx. + */ +export async function fetchOpenAiModelCatalog( + baseUrl: string, + apiKey: string, + fetchImpl: typeof fetch = fetch, +): Promise { + const url = `${baseUrl.replace(/\/+$/, "")}/models`; + const res = await fetchImpl(url, { + headers: { Authorization: `Bearer ${apiKey}` }, + signal: AbortSignal.timeout(CATALOG_FETCH_TIMEOUT_MS), + }); + if (!res.ok) throw new Error(`GET ${url} → ${res.status} ${res.statusText}`); + const body: unknown = await res.json(); + const data = (body as { data?: unknown })?.data; + if (!Array.isArray(data)) return []; + const out: ModelInfo[] = []; + for (const entry of data) { + const id = + typeof entry === "string" + ? entry + : typeof (entry as { id?: unknown })?.id === "string" + ? ((entry as { id: string }).id) + : null; + if (id) out.push({ id, displayName: id }); + } + return out; +} + +/** + * Union two catalogs, deduped by model id, `primary` order first. + * + * Where both sources carry the same id, the richer entry wins: an entry whose + * `displayName` differs from its id has a real human label behind it (the + * qwen-code registry supplies these; `/models` returns bare ids), so that one + * is kept and its description carried over. + * + * Exported for unit testing. + */ +export function unionCatalogs( + primary: readonly ModelInfo[], + secondary: readonly ModelInfo[], +): ModelInfo[] { + const labelled = (m: ModelInfo): boolean => m.displayName !== m.id; + const byId = new Map(); + for (const m of [...primary, ...secondary]) { + const existing = byId.get(m.id); + if (!existing) { + byId.set(m.id, m); + continue; + } + if (!labelled(existing) && labelled(m)) byId.set(m.id, m); + } + return [...byId.values()]; +} + /** * Registry servers for the qwen backend, as SDK MCP configs — a native mount, * since qwen-code owns its own MCP client. `${VAR}` env refs and diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 0aacdd8..b5fa542 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -1366,15 +1366,23 @@ mcpHub: this.#mcpHub, } /** - * Cache the live model catalog a provider reported. The list is - * version-static per provider within a daemon lifetime, so the first - * report per provider wins and we stop overwriting (cheap idempotence; - * avoids churn from every new session). + * Cache the live model catalog a provider reported. * - * The first report of each daemon lifetime is also persisted to SQLite - * (keyed by provider id), so subsequent boots serve current model names - * before any turn runs (see `#currentModels`) instead of the baked-in - * fallback that goes stale between codeoid releases. + * Every non-empty report wins, overwriting the previous one. Providers + * report on each query-loop build (i.e. session start), so a model added to + * a backend's gateway appears in the picker on the next session instead of + * waiting for a daemon restart — which matters now that the qwen backend + * resolves its catalog from a live `/models` call rather than a static list. + * An identical report is dropped before touching SQLite, so the common case + * (nothing changed since the last session) costs one comparison. + * + * EMPTY reports are ignored rather than cached: a backend that fails to + * answer must not clobber a good catalog with nothing. + * + * Each accepted report is persisted to SQLite (keyed by provider id), so + * subsequent boots serve current model names before any turn runs (see + * `#currentModels`) instead of the baked-in fallback that goes stale + * between codeoid releases. * * TypeScript-private (not `#`) so unit tests can exercise the persistence * path directly without a live backend query — same convention as @@ -1385,13 +1393,15 @@ mcpHub: this.#mcpHub, providerId: string, raw: ReadonlyArray<{ value: string; displayName: string; description?: string }>, ): void { - if (this.#modelsCache.has(providerId) || raw.length === 0) return; + if (raw.length === 0) return; const models = raw.map((m) => ({ value: m.value, displayName: m.displayName, ...(m.description ? { description: m.description } : {}), isDefault: m.value === "default", })); + const previous = this.#modelsCache.get(providerId); + if (previous && JSON.stringify(previous) === JSON.stringify(models)) return; this.#modelsCache.set(providerId, models); try { this.#store.saveModelCatalog(providerId, models); diff --git a/src/tests/models.test.ts b/src/tests/models.test.ts index f90e614..708f870 100644 --- a/src/tests/models.test.ts +++ b/src/tests/models.test.ts @@ -384,14 +384,26 @@ describe("models.list serves live → persisted → baked-in fallback, per provi expect(res.models.map((m) => m.value)).toEqual(["default", "fable", "opus"]); }); - it("first live report wins per provider for the lifetime; empty reports ignored", async () => { + // Providers report on every query-loop build, so the newest report is the + // most current view of what the backend serves — a model added to a gateway + // must appear on the next session, not after a daemon restart. + it("latest live report wins per provider; empty reports ignored", async () => { const manager = new SessionManager(store, new TranscriptStore(join(tmp, "t"))); const cache = manager as unknown as CacheModels; cache._cacheModels("claude", []); expect((await listModels(manager)).live).toBe(false); cache._cacheModels("claude", LIVE); cache._cacheModels("claude", [{ value: "other", displayName: "Other" }]); + expect((await listModels(manager)).models.map((m) => m.value)).toEqual(["other"]); + }); + + it("an empty report never clobbers an already-cached catalog", async () => { + const manager = new SessionManager(store, new TranscriptStore(join(tmp, "t"))); + const cache = manager as unknown as CacheModels; + cache._cacheModels("claude", LIVE); + cache._cacheModels("claude", []); const res = await listModels(manager); expect(res.models.map((m) => m.value)).toEqual(["default", "fable", "opus"]); + expect(res.live).toBe(true); }); }); diff --git a/src/tests/provider-qwen.test.ts b/src/tests/provider-qwen.test.ts index 4622bba..cf0e907 100644 --- a/src/tests/provider-qwen.test.ts +++ b/src/tests/provider-qwen.test.ts @@ -3,6 +3,8 @@ import { translateQwenMessage, resolveQwenAuthType, normalizeModelCatalog, + fetchOpenAiModelCatalog, + unionCatalogs, extractToolResultText, coerceBackingId, type QwenTranslateState, @@ -393,6 +395,30 @@ describe("normalizeModelCatalog", () => { ).toEqual([{ id: "qwen3.8-max", displayName: "Qwen3.8 Max", description: "flagship" }]); }); + // The shape @qwen-code/sdk 0.1.8 actually returns: `models` (not + // `availableModels`), `id` (not `modelId`), `label` (not `name`), and no + // description. Reading only `name` left every entry displaying its raw id. + test("reads the real sdk 0.1.8 shape — models[] with id + label", () => { + expect( + normalizeModelCatalog({ + subtype: "get_available_models", + models: [ + { id: "qwen3.8-max", label: "Qwen 3.8 Max", capabilities: {}, contextWindowSize: 1000000 }, + { id: "glm-5.2", label: "GLM 5.2", capabilities: {}, contextWindowSize: 1000000 }, + ], + }), + ).toEqual([ + { id: "qwen3.8-max", displayName: "Qwen 3.8 Max" }, + { id: "glm-5.2", displayName: "GLM 5.2" }, + ]); + }); + + test("falls back to the id when no label or name is present", () => { + expect(normalizeModelCatalog({ models: [{ id: "qwen3.8-flash" }] })).toEqual([ + { id: "qwen3.8-flash", displayName: "qwen3.8-flash" }, + ]); + }); + test("tolerates bare arrays, strings, and junk without throwing", () => { expect(normalizeModelCatalog(["a"])).toEqual([{ id: "a", displayName: "a" }]); expect(normalizeModelCatalog(null)).toEqual([]); @@ -401,6 +427,85 @@ describe("normalizeModelCatalog", () => { }); }); +describe("fetchOpenAiModelCatalog", () => { + function stubFetch(status: number, body: unknown): typeof fetch { + return (async (url: string | URL | Request, init?: RequestInit) => { + stubFetch.lastUrl = String(url); + stubFetch.lastAuth = (init?.headers as Record)?.Authorization; + return { + ok: status >= 200 && status < 300, + status, + statusText: String(status), + json: async () => body, + } as Response; + }) as unknown as typeof fetch; + } + stubFetch.lastUrl = ""; + stubFetch.lastAuth = ""; + + test("reads the OpenAI /models list shape", async () => { + const f = stubFetch(200, { + object: "list", + data: [ + { id: "qwen3.8-max", object: "model", owned_by: "system" }, + { id: "glm-5.2", object: "model", owned_by: "system" }, + ], + }); + expect(await fetchOpenAiModelCatalog("https://gw/v1", "sk-sp-x", f)).toEqual([ + { id: "qwen3.8-max", displayName: "qwen3.8-max" }, + { id: "glm-5.2", displayName: "glm-5.2" }, + ]); + expect(stubFetch.lastUrl).toBe("https://gw/v1/models"); + expect(stubFetch.lastAuth).toBe("Bearer sk-sp-x"); + }); + + test("strips trailing slashes off the base url", async () => { + await fetchOpenAiModelCatalog("https://gw/v1//", "k", stubFetch(200, { data: [] })); + expect(stubFetch.lastUrl).toBe("https://gw/v1/models"); + }); + + test("throws on non-2xx so the caller can fall back to the registry", async () => { + await expect( + fetchOpenAiModelCatalog("https://gw/v1", "bad", stubFetch(401, {})), + ).rejects.toThrow(/401/); + }); + + test("tolerates a missing or junk data array", async () => { + expect(await fetchOpenAiModelCatalog("https://gw/v1", "k", stubFetch(200, {}))).toEqual([]); + expect( + await fetchOpenAiModelCatalog("https://gw/v1", "k", stubFetch(200, { data: [{ no: 1 }] })), + ).toEqual([]); + }); +}); + +describe("unionCatalogs", () => { + test("dedupes by id and keeps the entry that has a real label", () => { + // /models returns bare ids; the qwen-code registry supplies labels. + const live = [{ id: "qwen3.8-max", displayName: "qwen3.8-max" }, { id: "glm-5.2", displayName: "glm-5.2" }]; + const registry = [{ id: "qwen3.8-max", displayName: "Qwen 3.8 Max" }]; + expect(unionCatalogs(live, registry)).toEqual([ + { id: "qwen3.8-max", displayName: "Qwen 3.8 Max" }, + { id: "glm-5.2", displayName: "glm-5.2" }, + ]); + }); + + test("keeps registry-only models the gateway never reported", () => { + // qwen-oauth's built-in `coder-model` has no /models endpoint behind it. + expect( + unionCatalogs([], [{ id: "coder-model", displayName: "coder-model" }]), + ).toEqual([{ id: "coder-model", displayName: "coder-model" }]); + }); + + test("preserves primary ordering", () => { + expect( + unionCatalogs( + [{ id: "a", displayName: "a" }, { id: "b", displayName: "b" }], + [{ id: "b", displayName: "B!" }, { id: "c", displayName: "c" }], + ).map((m) => m.id), + ).toEqual(["a", "b", "c"]); + }); +}); + describe("extractToolResultText", () => { test("handles strings, block arrays, and images", () => { expect(extractToolResultText("plain")).toBe("plain");