#41 List models from the server for OpenAI-compatible providers

[Anonymous]

Originally created by: sachin-detrax

Closes [#31].

Problem

The OpenAI-compatible provider wizard asked the operator to type a chat model id by hand. Against a remote vLLM/SGLang deployment — the case in [#31] — nobody knows the ids offhand, and providers rotate them without notice. The server already publishes them at GET {baseUrl}/v1/models.

Change

• src/llm/provider/openai/fetch-openai-compat-models.ts (new) — GET {baseUrl}/v1/models with the bearer key, sorted ids, 10s timeout, cached per normalized base URL. Same fetch-then-read-sync shape the OpenRouter picker already uses, so the sync key handler can read the result.
• Chat model step — now fetches on entry and renders the ids as a 12-row window around the cursor (↑/↓ move (21/341) · Enter select). Typing still works: any printable key is a deliberate override that hides the list, backspacing to empty brings it back. Arrows only for movement, so ids starting with j/k stay typable.
• Failure is not fatal — a refused, empty or unreachable endpoint degrades to the old typed field with the reason in the hint (model list unavailable (http 401) — type the id).
• Base URL normalization on save — a base pasted as https://host/v1/ was stored verbatim and produced https://host/v1//v1/chat/completions on every later request. It is now normalized once, and the same helper feeds the fetch, the cache key and the displayed URL.

The API key comes from the wizard buffer, falling back to resolveLlmProviderApiKey (OPENAI_COMPAT_API_KEY / OPENAI_API_KEY / ATOMIC_AGENT_OPENAI_API_KEY), so a key already in .env needs no retyping.

Verification

15 new tests (fetch/cache/normalize, picker key flow, typed override, windowed render, refusal fallback). Full suite: 3293 passing, unchanged pre-existing failures on main; tsc clean.

Also exercised against real servers, no mocks:

• https://openrouter.ai/api — 341 ids listed and rendered; base pasted as .../api/v1/ hit the cache instead of re-requesting; arrows + Enter selected a model and built the entry with baseUrl: https://openrouter.ai/api.
• A local vLLM-shaped HTTP server — server log showed exactly one GET /v1/models auth=Bearer <key>, and the cold-cache render produced the windowed picker.
• https://api.openai.com with no key → http 401, nothing cached, hint shown; unreachable host → fetch failed hint.

Known limitation

Servers whose OpenAI base is a path prefix rather than a host root (e.g. DeepInfra's https://api.deepinfra.com/v1/openai) 404 on discovery, because the codebase appends /v1/... and the normalizer only strips a trailing /v1. That shape already cannot serve chat completions through this provider, so it is pre-existing rather than a regression; the wizard falls back to typed entry there.

[Anonymous]

Originally posted by: Ooooze

Thanks for this — the wizard genuinely needed it, and the degradation path when the server refuses the list is handled honestly.

I checked the wizard-state assumption this builds on: cursor was previously a pick-list index only, and advanceWizardPhase resets it to 0 on every phase transition, so reusing it in chat_model_line is safe. No issue there.

Four things I'd like fixed before merge, roughly in order of importance.

1. Base URL normalization belongs in the provider, not the wizard.

OpenAiProvider only strips a trailing slash today:

// src/llm/provider/openai/openai-provider.ts
this.http = { baseUrl: options.baseUrl.replace(/\/$/, ""), ... }

and every call site appends /v1/.... Normalizing in buildProviderEntryFromWizard fixes the wizard path only — configs written before this PR, or edited by hand in config.json, keep producing https://host/v1/v1/chat/completions. Moving (or duplicating) the normalization into OpenAiProvider / register-built-in-providers fixes it for everyone and makes the wizard's job just "store what the operator typed".

2. /\/v\d+$/ is broader than the stated intent.

The comment says "a base URL pasted with /v1", but the regex strips any /vN. https://host/api/v2 silently becomes https://host/api, and the next request goes to /api/v1/.... Please anchor it to /v1.

3. The cache has no TTL and ignores the API key.

cache.set(base, ids) is never invalidated, so a rotated key or a newly-deployed model is invisible until the process restarts — and a list fetched anonymously is reused for a later authenticated request. The OpenRouter analogue (fetch-openrouter-chat-catalog.ts) already carries a 1h TTL; matching that, plus folding the key into the cache key, would cover both cases.

Related, and more of a judgement call for a maintainer: reading this module-level Map from handleProvidersWizardKey turns a previously pure reducer into one with a hidden global dependency (the new tests have to prime the cache through a stubbed fetch to exercise it). The OpenRouter precedent exists, so I won't block on it, but it's worth a conscious call.

4. Minor.

• The useEffect isn't guarded by w.kind — only listCompatChatModelPicks is. Harmless today since openai-compatible is the sole non-curated kind routed through base_url, but a fourth kind would start firing a request at the default https://api.openai.com with whatever key resolves from the environment. A kind check in the effect keeps that from happening later.
• normalizeOpenAiCompatBaseUrl lives inside the fetch module, but providers-wizard-build-entry.ts imports it purely as a URL helper. Per the repo's one-responsibility-per-file rule it probably deserves its own file (this becomes moot if it moves to the provider layer per point 1).

[Anonymous]

Originally posted by: sachin-detrax

Thanks — all four fixed in b2c3598.

1 + 2. Normalization moved to the provider, anchored to /v1.

New src/llm/provider/openai/normalize-openai-base-url.ts, called from OpenAiProvider's constructor in place of the old replace(/\/$/, ""). Configs written before this PR or hand-edited into config.json are now fixed too, and the wizard's job is back to "store what the operator typed". Only a trailing /v1 is stripped, so https://host/api/v2 is left alone.

Worth noting the duplication your point uncovered: normalizeOpenRouterBaseUrl and normalizeAimlapiBaseUrl were already two copies of exactly this logic. Both are now re-exports of the shared helper, and neither subclass normalizes on its own any more since the base class does it.

3. Cache has a 1h TTL and is keyed by base URL + api key.

Matches fetch-openrouter-chat-catalog.ts. Entries store fetchedAt and the key is `${base}\n${apiKey ?? ""}`, so a rotated key or a newly deployed model is visible without a restart, and an anonymous list can no longer serve an authenticated request. The key resolution the effect was doing inline is now apiKeyForWizard(), exported next to baseUrlForWizard(), so the async fetch and the sync cache read cannot disagree about which key they mean.

I left the module-level Map as-is — that's the maintainer call you flagged, and the OpenRouter precedent is what the picker's fetch-then-read-sync shape is built on.

4.

• The effect now returns early unless w.kind === "openai-compatible", and the hint falls back to the plain line-field text for any other kind.
• normalizeOpenAiCompatBaseUrl is gone from the fetch module; it lives in its own file per point 1, which is what providers-wizard-build-entry.ts imports.

Tests. Three new in fetch-openai-compat-models.test.ts (key isolation, TTL expiry with fake timers) plus normalize-openai-base-url.test.ts covering the /api/v2 and /v10 cases. The key-bindings tests now pin OPENAI_COMPAT_API_KEY instead of reading whatever is in the ambient env, since the cache key depends on it.

tsc clean. Full suite 3287 passing; the failing files are the same set as main — parallel-tool-calls.integration and llm-health-poller pass in isolation and vary run to run on both branches.

[Anonymous]

Ticket changed by: Ooooze

• status: open --> closed