From 6e98573f24a09c858bbdfcf1171e39f37c2bc3d1 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Wed, 10 Jun 2026 23:30:04 +0200 Subject: [PATCH] feat(ai): sync model reads, explicit async refresh Provider.getModels() is sync-only (last-known list; must not throw) with an optional refreshModels() where dynamic providers fetch. The sync-or-async union invited latent sync assumptions that would detonate on the first dynamic provider; async-only reads would force sync consumer surfaces (extension find/getAll) through Promises. Sync reads plus an explicit refresh verb keeps the contract single and the staleness visible. Models.getModels()/getModel() are sync best-effort reads; Models.refresh(provider?) rejects with ModelsError(model_source) for a single provider and is concurrent best-effort across all providers. createProvider() takes a models array plus an optional refreshModels fetcher (stored on success, in-flight calls deduped, list unchanged on rejection). forceRefresh options are gone. Also finishes the in-progress AuthStorage fallbackResolver removal (drops the now-unused includeFallback option from getApiKey). --- packages/agent/docs/models.md | 98 ++++++++++----- packages/ai/CHANGELOG.md | 4 +- packages/ai/src/models.ts | 118 ++++++++++++------ packages/ai/test/lazy-module-load.test.ts | 2 +- packages/ai/test/models-runtime.test.ts | 87 +++++++------ packages/ai/test/oauth-auth.test.ts | 4 +- packages/ai/test/providers.test.ts | 35 ++++-- packages/ai/test/scratch.ts | 2 +- .../coding-agent/src/core/auth-storage.ts | 22 +--- .../coding-agent/src/core/model-registry.ts | 4 +- 10 files changed, 237 insertions(+), 139 deletions(-) diff --git a/packages/agent/docs/models.md b/packages/agent/docs/models.md index f5cb00cea..cfa524e0a 100644 --- a/packages/agent/docs/models.md +++ b/packages/agent/docs/models.md @@ -10,7 +10,7 @@ Goals: - Concrete provider factories live under `src/providers/`. - Users can import only the providers they need. - Importing a provider must not eagerly import heavy SDKs. -- Dynamic model lists are first-class and side-effect-free. +- Dynamic model lists are first-class: reads are sync (last-known list), fetching happens in an explicit async `refresh`. - `models.json` and extensions layer by wrapping providers, not by mutating provider internals ad hoc. - Old global APIs survive only in an explicit, temporary `/compat` entrypoint. @@ -129,11 +129,17 @@ export interface Models { getProviders(): readonly Provider[]; getProvider(id: string): Provider | undefined; - /** Best-effort aggregation: provider source failures yield the models that did list. */ - getModels(options?: { forceRefresh?: boolean }): Promise[]>; - getModels(provider?: string, options?: { forceRefresh?: boolean }): Promise[]>; + /** Sync read of last-known models. Best-effort: a provider whose getModels() throws yields no models. */ + getModels(provider?: string): readonly Model[]; /** Dynamic lists are honestly Model; narrow with the hasApi() guard. */ - getModel(provider: string, id: string, options?: { forceRefresh?: boolean }): Promise | undefined>; + getModel(provider: string, id: string): Model | undefined; + + /** + * Ask dynamic providers to re-fetch their model lists. With a provider id, + * rejects on that provider's failure; without, refreshes all concurrently + * best-effort. Static providers are no-ops. + */ + refresh(provider?: string): Promise; /** * Resolve request auth for a model. Includes source label for status UI. @@ -201,8 +207,11 @@ export interface Provider { */ readonly auth: ProviderAuth; - /** Sync return suits static catalogs; Models always exposes a Promise. */ - getModels(options?: { forceRefresh?: boolean }): Promise[]> | readonly Model[]; + /** Current known models, sync. Static providers: the catalog. Dynamic providers: as of the last refresh (empty before the first). */ + getModels(): readonly Model[]; + + /** Dynamic providers only: fetch and update the model list. Concurrent calls share one in-flight fetch. */ + refreshModels?(): Promise; stream(model: Model, context: Context, options?: ApiStreamOptions): AssistantMessageEventStream; @@ -268,16 +277,20 @@ For comparison: Vercel AI SDK attaches the implementation to the model object, w ## Provider model listing -`Provider.getModels()` is async and returns full `Model` objects. Static providers wrap their catalog; dynamic providers (llama.cpp, OpenRouter live listing) fetch and cache, honoring `forceRefresh`. +Reads are sync; fetching is an explicit async verb. `Provider.getModels()` returns the current known list — the full catalog for static providers, the last-refreshed list for dynamic ones (llama.cpp, OpenRouter live listing). `refreshModels()` is where dynamic providers fetch. -Dynamic model listing must be side-effect-free discovery: +This split exists because a sync-or-async union (`Promise | T`) invites latent sync assumptions that detonate on the first async provider, while async-only reads force every consumer (UI lists, extension `find`/`getAll` surfaces) through Promises for data that is almost always static. Sync reads + explicit refresh keeps the staleness visible and the contract single: `getModels()` = last known, `refresh()` = make it current. A fetched list is stale the moment it returns anyway; naming the refresh point is honest about it. + +Apps own the refresh lifecycle: startup, registry reload, opening a model selector. Freshness-critical lookups are two-step: `await models.refresh("llamacpp"); models.getModel("llamacpp", id)`. + +Dynamic refresh must be side-effect-free discovery: ```txt OK: fetch /v1/models, enumerate local catalog, refresh cached remote model list Not OK: load model, download model, mutate server state, run request probe ``` -Provider-specific model lifecycle (load/unload) belongs in app/provider-management commands, not in `getModels()`. +Provider-specific model lifecycle (load/unload) belongs in app/provider-management commands, not in `refreshModels()`. ## Streaming path @@ -301,7 +314,7 @@ function stream(model, context, options) { `stream()` returns `AssistantMessageEventStream` synchronously; async setup (auth resolution, lazy module load) happens inside the returned stream. The forwarding pattern already exists in today's `register-builtins.ts` (`createLazyStream`); extract it as `lazyStream()` in `src/api/lazy.ts`. -No request hot-path model canonicalization: `stream()` uses the supplied model object as-is. If an app wants fresh model metadata, it calls `models.getModel(provider, id, { forceRefresh: true })` before starting the turn. +No request hot-path model canonicalization: `stream()` uses the supplied model object as-is. If an app wants fresh model metadata, it refreshes the provider and re-reads (`await models.refresh(p); models.getModel(p, id)`) before starting the turn. ## API implementations under `src/api` @@ -629,10 +642,8 @@ function withProviderOverrides(base: Provider, overrides: ProviderOverrides): Pr baseUrl: overrides.baseUrl ?? base.baseUrl, headers: mergeHeaders(base.headers, overrides.headers), - async getModels(options) { - const models = await base.getModels(options); - return applyModelOverrides(models, overrides.models); - }, + getModels: () => applyModelOverrides(base.getModels(), overrides.models), + refreshModels: base.refreshModels?.bind(base), stream: base.stream, streamSimple: base.streamSimple, @@ -640,7 +651,7 @@ function withProviderOverrides(base: Provider, overrides: ProviderOverrides): Pr } ``` -This composes with dynamic providers because `getModels()` delegates to the base source. +This composes with dynamic providers because `getModels()` delegates to the base source and `refreshModels()` passes through. Request-auth config from models.json (`$ENV`, `!command`, inline keys) remains app-owned sidecar state, surfaced either as explicit request auth or as a custom `ApiKeyAuth` the app sets on the wrapped provider's `auth.apiKey`. @@ -655,9 +666,10 @@ export function createProvider(input: { baseUrl?: string; headers?: Record; auth: ProviderAuth; // required, at least one of apiKey/oauth (no "no-auth" providers) - models: - | readonly Model[] - | ((options?: { forceRefresh?: boolean }) => Promise[]>); + /** Initial model list (empty for purely dynamic providers). */ + models: readonly Model[]; + /** Dynamic providers: fetch the current list; createProvider stores it and dedupes in-flight calls. */ + refreshModels?: () => Promise[]>; /** Single implementation, or map keyed by model.api for mixed-API providers. */ api: ProviderStreams | Record; }): Provider; @@ -837,17 +849,45 @@ Check items off as they land. Keep this list current; it is the working state fo - [x] `packages/agent/CHANGELOG.md`: `### Breaking Changes` for required `AgentHarnessOptions.models`, compaction signature changes, structural `StreamFn`. - [x] `npm run check` clean. -### Phase 9 — ModelManager migration (separate pass, not started) +### Phase 9 — coding-agent on Models + CredentialStore (in scope) -The big one: coding-agent moves off ModelRegistry/AuthStorage onto `Models` + `CredentialStore`, and compat dies. Ordering sketch: +coding-agent replaces AuthStorage and ModelRegistry's internals with `FileCredentialStore` + a `MutableModels` collection. AgentSession itself stays (AgentHarness adoption is pi 2.0); only its model/auth substrate swaps. Layering is strictly one-directional: -- [ ] coding-agent constructs a `Models` instance per session: builtins (`builtinModels()`), models.json custom providers via `createProvider()` + `withProviderOverrides`, extension custom providers as real providers (legacy `registerApiProvider` extensions bridged by a catch-all api-dispatch provider until extensions migrate). -- [ ] `AgentSession` streams through that instance (directly or by adopting `AgentHarness`); `agent.streamFn` identity checks and env-key injection die. -- [ ] AuthStorage replaced per "Replacing AuthStorage": `FileCredentialStore` (ports the lock backend), `withConfigValues` (`$ENV`/`!command`), `withRuntimeOverrides` (`--api-key`); custom providers carry their own `ApiKeyAuth` (kills `fallbackResolver`). -- [ ] Login/logout/status UIs move to `ProviderAuth` (`OAuthAuth.login` + `prompt()/notify()` adapter in the login dialog); the old `pi-ai/oauth` registry and `OAuthProviderInterface` (incl. `usesCallbackServer`) are deleted. -- [ ] Cloudflare cleanup (gated on builtin streaming going through `Models.getAuth`): cloudflare factories' `ApiKeyAuth.resolve` reads key + `CLOUDFLARE_ACCOUNT_ID` (+ `CLOUDFLARE_GATEWAY_ID`) from credential metadata/env, substitutes the `{...}` placeholders in `model.baseUrl`, and returns `ModelAuth.baseUrl` (Copilot pattern); unconfigured ids report "not configured". `resolveCloudflareBaseUrl`/`isCloudflareProvider` drop out of `api/anthropic-messages.ts`, `api/openai-completions.ts`, `api/openai-responses.ts`. +```txt +FileCredentialStore (auth.json, locked) + --api-key overlay + $ENV/!command resolution + ↑ +MutableModels: builtin factories (wrapped per models.json config) + custom providers (models.json ∪ extensions) + ↑ +ModelRegistry: async facade — reads delegate to the collection; registerProvider/login/logout/status for extensions + UI + ↑ +AgentSession / sdk / interactive-mode (await added; stream via models) +``` + +Decisions: + +- `AuthStorage` is deleted as a type — it would otherwise depend on provider auth while provider auth depends on its store (circular). Its surface splits: `get`/`set`/`remove` -> `CredentialStore`; `getApiKey` -> `Models.getAuth`; `login`/`logout`/`getAuthStatus` -> ModelRegistry facade methods over `provider.auth.oauth` + the store. +- Runtime `--api-key` overrides are a store overlay (an override reads as an ephemeral stored api-key credential, masking stored OAuth — matches today's priority). Every registered provider is guaranteed an `apiKey` auth slot so overrides apply to OAuth-only providers too. +- `ModelRegistry.getAll`/`find`/`getAvailable` become async, delegating to the collection (no materialized snapshot, no sync lies; dynamic providers like llama.cpp work). The extension-facing `modelRegistry` surface changes accordingly (breaking, changelogged); extensions also get the collection itself as the forward API. +- models.json keeps FULL feature parity, implemented as provider decoration: builtin factories wrapped so `getModels()` applies provider `baseUrl`/`compat` overlays, `modelOverrides`, and custom-model merges (async-safe); provider `apiKey`/`headers`/`authHeader` configs become that provider's `ApiKeyAuth` (config first, factory auth fallback); parse errors keep `getError()` semantics. +- Extension `ProviderConfig` parity: provider-keyed `streamSimple`, old-style `oauth` adapted to `OAuthAuth` (`modifyModels` -> `getModels` wrap + `toAuth`), full model replacement per provider. The api-registry `registerApiProvider` dual-write stays for compat consumers (extensions calling global `complete()`); it dies with compat. +- Copilot: stored-credential baseUrl applied in the wrapped `getModels()` (extension-visible models stay correct) plus per-request `toAuth().baseUrl`. +- Cloudflare: provider-auth substitution (key + `CLOUDFLARE_ACCOUNT_ID`/`CLOUDFLARE_GATEWAY_ID` from credential metadata/env -> `ModelAuth.baseUrl`); impl-side `resolveCloudflareBaseUrl` stays until compat dies (it is idempotent on substituted URLs), keeping extension `complete()` calls working. + +Ordering: + +- [ ] pi-ai rework first: `Provider.getModels()` sync + optional `refreshModels()`; `Models.getModels`/`getModel` sync, `Models.refresh(provider?)` async; `createProvider` takes `models` array + optional `refreshModels` fetcher (in-flight dedupe). Reverses Phase 1's async-listing decision — see "Provider model listing" for rationale (sync-or-async unions breed latent sync assumptions; async-only breaks sync consumer surfaces like extension `find`/`getAll`). +- [ ] `FileCredentialStore` (ports the auth.json lock backend, reads legacy `type: "api_key"` tags) + `--api-key` overlay + `$ENV`/`!command` resolution; tests. +- [ ] Cloudflare provider auth in pi-ai factories; copilot `getModels` baseUrl wrap. +- [ ] Extension-OAuth adapter (old `OAuthProviderInterface` config -> `OAuthAuth`). +- [ ] ModelRegistry rebuild: owns `MutableModels`, async reads, models.json decoration, provider-keyed extension streams, facade auth ops; AuthStorage deleted. +- [ ] Consumer rewiring: agent-session, sdk (`credentials?: CredentialStore` option replaces `authStorage`; sdk.md updated), model-resolver, interactive login/status UI on `prompt()/notify()`, cli `--api-key`. +- [ ] Test migration; tmux validation of login flows against real providers. + +### Phase 10 — compat deletion (pi 2.0 era, separate) + +- [ ] AgentSession -> AgentHarness; the registry facade dies in favor of harness `Models`. - [ ] Move ALL internal `/compat` imports to the new API: every package's src, all tests, and the example extensions (examples then demonstrate the new API). Nothing inside the repo may import `/compat` at that point. -- [ ] Delete `/compat`, `api-registry.ts`, `env-api-keys.ts`, and the extension-loader root-to-compat alias. This is the extension-author breaking release; changelog carries the migration guide. +- [ ] Delete `/compat`, `api-registry.ts`, `env-api-keys.ts`, the extension-loader root-to-compat alias, the old `pi-ai/oauth` registry and `OAuthProviderInterface` (incl. `usesCallbackServer`), and the impl-side cloudflare substitution. This is the extension-author breaking release; changelog carries the migration guide. ### Deferred / follow-ups @@ -860,7 +900,7 @@ The big one: coding-agent moves off ModelRegistry/AuthStorage onto `Models` + `C ```ts export type ModelsErrorCode = - | "model_source" // provider getModels() failed + | "model_source" // provider model refresh failed | "model_validation" // model object invalid | "provider" // unknown provider, dispatch failure | "stream" // stream setup failure @@ -869,6 +909,6 @@ export type ModelsErrorCode = ``` - `Models.stream()` produces stream errors (error event + error result) for async setup failures; it does not throw after returning the stream. -- `Models.getModels()` is best-effort aggregation in all forms: provider source failures yield the models that did list (empty for a single failing provider). Apps that need the concrete failure call `getProvider(id).getModels()` directly. +- `Models.getModels()` is a sync best-effort read: a provider whose `getModels()` throws yields no models. `Models.refresh(provider)` rejects on that provider's fetch failure; `Models.refresh()` (all providers) is concurrent best-effort. Apps that need a concrete listing failure refresh the single provider. - Auth resolution and credential store failures reject loudly (`ModelsError` codes `auth`/`oauth`); silent fallback to a different auth path after a failure risks billing surprises. A stored credential always blocks ambient/env fallback, including after a failed refresh. - Status/availability UIs catch `getAuth` rejections and render "needs re-login"; they do not treat rejection as "unconfigured". diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 101fabf27..0c0d3bd6b 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -4,13 +4,13 @@ ### Breaking Changes -- The root entrypoint (`@earendil-works/pi-ai`) is now core-only and side-effect free. The old global API moved to the temporary `@earendil-works/pi-ai/compat` entrypoint, a strict superset of the root: switching a file's import path is the only migration step. Moved symbols include `stream`/`complete`/`streamSimple`/`completeSimple`, `getModel`/`getModels`/`getProviders` (now deprecated aliases of `getBuiltinModel`/`getBuiltinModels`/`getBuiltinProviders` from `@earendil-works/pi-ai/providers/all`), `registerApiProvider`/`unregisterApiProviders`/`resetApiProviders`/`getApiProvider`, `getEnvApiKey`/`findEnvKeys`, `setBedrockProviderModule`, the per-API lazy stream wrappers (`anthropicMessagesApi`, ...), and the image-generation API. `/compat` will be removed with the coding-agent ModelManager migration; new code uses `createModels()` and the provider factories instead. +- The root entrypoint (`@earendil-works/pi-ai`) is now core-only and side-effect free. The old global API moved to the temporary `@earendil-works/pi-ai/compat` entrypoint, a strict superset of the root: switching a file's import path is the only migration step. Moved symbols include `stream`/`complete`/`streamSimple`/`completeSimple`, `getModel`/`getModels`/`getProviders` (now deprecated aliases of `getBuiltinModel`/`getBuiltinModels`/`getBuiltinProviders` from `@earendil-works/pi-ai/providers/all`), `registerApiProvider`/`unregisterApiProviders`/`resetApiProviders`/`getApiProvider`, `getEnvApiKey`/`findEnvKeys`, `setBedrockProviderModule`, the per-API lazy stream wrappers (`anthropicMessagesApi`, ...), and the image-generation API. - Renamed the `Provider` type to `ProviderId`. `Provider` now names the runtime provider interface (id, name, auth, model listing, stream behavior). - API implementation modules moved from `src/providers/` to `@earendil-works/pi-ai/api/*`, renamed by API id (`anthropic` -> `api/anthropic-messages`, `google` -> `api/google-generative-ai`, `mistral` -> `api/mistral-conversations`, `amazon-bedrock` -> `api/bedrock-converse-stream`), each exporting exactly `stream` and `streamSimple`. The old per-impl export names (`streamAnthropic`, `streamSimpleAnthropic`, ...) are gone; the legacy package subpaths (`./anthropic`, `./google`, ...) keep working and point at the new modules. ### Added -- New `Models` runtime: `createModels()` builds an isolated provider collection with async model listing, auth resolution (`getAuth`), and `stream`/`complete`/`streamSimple`/`completeSimple` that resolve auth through the owning provider. `createProvider()` builds providers from parts (single API implementation or a map dispatched on `model.api`); `hasApi()` narrows dynamically listed models. +- New `Models` runtime: `createModels()` builds an isolated provider collection with sync model reads (`getModels`/`getModel` return the last-known lists), an explicit async `refresh(provider?)` for dynamic providers, auth resolution (`getAuth`), and `stream`/`complete`/`streamSimple`/`completeSimple` that resolve auth through the owning provider. `createProvider()` builds providers from parts (single API implementation or a map dispatched on `model.api`; static `models` array plus an optional `refreshModels` fetcher with in-flight dedupe); `hasApi()` narrows dynamically listed models. - Provider auth substrate: `ProviderAuth` (`{ apiKey?, oauth? }`), one type-tagged credential per provider, `CredentialStore` (`read`/`modify`/`delete` with serialized writes; in-memory default), `envApiKeyAuth()`, `lazyOAuth()`, and injectable `AuthContext`. OAuth refresh runs under the store lock with double-checked expiry; a stored credential owns its provider (no silent env fallback). - One provider factory per built-in provider under `@earendil-works/pi-ai/providers/*` (e.g. `anthropicProvider()`, `openrouterProvider()`), plus `@earendil-works/pi-ai/providers/all` with `builtinProviders()`/`builtinModels()` and typed `getBuiltin*` catalog reads. Generated catalogs are split per provider, so importing one provider pulls one catalog; `sideEffects` metadata makes the package tree-shakeable. - OAuth flows (Anthropic, OpenAI Codex, GitHub Copilot) gained `OAuthAuth` adapters (`login`/`refresh`/`toAuth`) on unified `prompt()`/`notify()` login callbacks; Copilot's per-credential base URL is derived in `toAuth()`. diff --git a/packages/ai/src/models.ts b/packages/ai/src/models.ts index 6cb7a87d0..37d338411 100644 --- a/packages/ai/src/models.ts +++ b/packages/ai/src/models.ts @@ -64,10 +64,21 @@ export interface Provider { readonly auth: ProviderAuth; /** - * List models. Async and side-effect-free discovery only; provider-specific - * model lifecycle (load/unload) belongs in app commands. + * Current known models, sync. Static providers return their catalog; + * dynamic providers return the list as of the last `refreshModels()` + * (empty before the first). Must not throw; `Models` treats a throwing + * implementation as having no models. */ - getModels(options?: { forceRefresh?: boolean }): Promise[]> | readonly Model[]; + getModels(): readonly Model[]; + + /** + * Dynamic providers only: fetch and update the model list. Side-effect-free + * discovery (no loading/downloading); provider-specific model lifecycle + * belongs in app commands. Concurrent calls share one in-flight fetch. + * May reject (network); on rejection the model list stays at its last-known + * state and a later call retries. + */ + refreshModels?(): Promise; stream( model: Model, @@ -88,19 +99,24 @@ export interface Models { getProvider(id: string): Provider | undefined; /** - * List models from one provider or all providers. Best-effort aggregation: - * provider source failures yield the models that did list (empty for a - * single failing provider). Apps that need the failure call - * `getProvider(id).getModels()` directly. + * Sync read of last-known models from one provider or all providers. + * Best-effort: a provider whose `getModels()` throws yields no models. */ - getModels(options?: { forceRefresh?: boolean }): Promise[]>; - getModels(provider?: string, options?: { forceRefresh?: boolean }): Promise[]>; + getModels(provider?: string): readonly Model[]; /** - * Runtime model lookup. Dynamic model lists are typed as `Model`; - * narrow with the `hasApi()` type guard. + * Sync runtime model lookup against last-known lists. Dynamic model lists + * are typed as `Model`; narrow with the `hasApi()` type guard. */ - getModel(provider: string, id: string, options?: { forceRefresh?: boolean }): Promise | undefined>; + getModel(provider: string, id: string): Model | undefined; + + /** + * Ask dynamic providers to re-fetch their model lists. With a provider id, + * rejects with `ModelsError` ("model_source") on that provider's fetch + * failure; without one, refreshes all providers concurrently best-effort. + * Static providers (no `refreshModels`) are no-ops. + */ + refresh(provider?: string): Promise; /** * Resolve request auth for a model. Includes a source label for status UI. @@ -171,37 +187,48 @@ class ModelsImpl implements MutableModels { return this.providers.get(id); } - async getModels( - providerOrOptions?: string | { forceRefresh?: boolean }, - maybeOptions?: { forceRefresh?: boolean }, - ): Promise[]> { - const provider = typeof providerOrOptions === "string" ? providerOrOptions : undefined; - const options = typeof providerOrOptions === "string" ? maybeOptions : providerOrOptions; - + getModels(provider?: string): readonly Model[] { if (provider !== undefined) { const entry = this.providers.get(provider); if (!entry) return []; try { - return await entry.getModels(options); + return entry.getModels(); } catch { return []; } } - // Async wrapper turns sync throws from ill-behaved providers into rejections. - const results = await Promise.allSettled( - Array.from(this.providers.values(), async (entry) => entry.getModels(options)), - ); const models: Model[] = []; - for (const result of results) { - if (result.status === "fulfilled") models.push(...result.value); + for (const entry of this.providers.values()) { + try { + models.push(...entry.getModels()); + } catch { + // Best-effort: ill-behaved providers yield no models. + } } return models; } - async getModel(provider: string, id: string, options?: { forceRefresh?: boolean }): Promise | undefined> { - const models = await this.getModels(provider, options); - return models.find((model) => model.id === id); + getModel(provider: string, id: string): Model | undefined { + return this.getModels(provider).find((model) => model.id === id); + } + + async refresh(provider?: string): Promise { + if (provider !== undefined) { + const entry = this.providers.get(provider); + if (!entry?.refreshModels) return; + try { + await entry.refreshModels(); + } catch (error) { + if (error instanceof ModelsError) throw error; + throw new ModelsError("model_source", `Model refresh failed for ${provider}`, { cause: error }); + } + return; + } + + // Cannot reject: the async mapper turns even sync throws from ill-behaved + // providers into rejections, and allSettled captures all of them. + await Promise.allSettled(Array.from(this.providers.values(), async (entry) => entry.refreshModels?.())); } async getAuth(model: Model): Promise { @@ -358,9 +385,16 @@ export interface CreateProviderOptions { headers?: Record; /** Required — every provider has auth semantics, even ambient/keyless ones. */ auth: ProviderAuth; - models: - | readonly Model[] - | ((options?: { forceRefresh?: boolean }) => Promise[]> | readonly Model[]); + /** Initial model list (empty for purely dynamic providers). */ + models: readonly Model[]; + /** + * Dynamic providers: fetch the current list. Stored on success; concurrent + * calls share one in-flight fetch. May reject: the stored list then stays + * at its last-known state, the rejection propagates to the caller of + * `refreshModels()` (wrapped as ModelsError "model_source" by + * `Models.refresh(provider)`), and a later call retries. + */ + refreshModels?: () => Promise[]>; /** Single implementation, or map keyed by `model.api` for mixed-API providers. */ api: ProviderStreams | Partial>; } @@ -372,7 +406,9 @@ export interface CreateProviderOptions { * produces a stream error. */ export function createProvider(input: CreateProviderOptions): Provider { - const { models } = input; + let models = input.models; + let inflightRefresh: Promise | undefined; + const refreshModels = input.refreshModels; const single = typeof (input.api as ProviderStreams).stream === "function" ? (input.api as ProviderStreams) : undefined; const byApi = single ? undefined : (input.api as Partial>); @@ -398,7 +434,19 @@ export function createProvider(input: CreateProviderOpti baseUrl: input.baseUrl, headers: input.headers, auth: input.auth, - getModels: typeof models === "function" ? (options) => models(options) : () => models, + getModels: () => models, + refreshModels: refreshModels + ? () => { + inflightRefresh ??= (async () => { + try { + models = await refreshModels(); + } finally { + inflightRefresh = undefined; + } + })(); + return inflightRefresh; + } + : undefined, stream: (model, context, options) => dispatch(model, (streams) => streams.stream(model, context, options)), streamSimple: (model, context, options) => dispatch(model, (streams) => streams.streamSimple(model, context, options)), @@ -409,7 +457,7 @@ export function createProvider(input: CreateProviderOpti * Runtime-checked narrowing for dynamically looked-up models: * * ```ts - * const model = await models.getModel("anthropic", "claude-opus-4-7"); + * const model = models.getModel("anthropic", "claude-opus-4-7"); * if (model && hasApi(model, "anthropic-messages")) { * // model: Model<"anthropic-messages">, stream options fully typed * } diff --git a/packages/ai/test/lazy-module-load.test.ts b/packages/ai/test/lazy-module-load.test.ts index 6f5464ff9..dd5169625 100644 --- a/packages/ai/test/lazy-module-load.test.ts +++ b/packages/ai/test/lazy-module-load.test.ts @@ -72,7 +72,7 @@ describe("lazy provider module loading", () => { const result = runProbe(` const all = await import(${JSON.stringify(providersAllUrl)}); const models = all.builtinModels(); - await models.getModels(); + models.getModels(); `); expect(result.loadedSpecifiers).toEqual([]); }); diff --git a/packages/ai/test/models-runtime.test.ts b/packages/ai/test/models-runtime.test.ts index 7f94ccb78..cafcb17fe 100644 --- a/packages/ai/test/models-runtime.test.ts +++ b/packages/ai/test/models-runtime.test.ts @@ -55,7 +55,8 @@ function testProvider(input: { id: string; models?: Model[]; auth?: ProviderAuth; - getModels?: () => Promise[]>; + getModels?: () => readonly Model[]; + refreshModels?: () => Promise; calls?: ProviderCall[]; }): Provider { const models = input.models ?? [testModel(input.id, "model-a")]; @@ -72,7 +73,8 @@ function testProvider(input: { id: input.id, name: input.id, auth: input.auth ?? { apiKey: ambientAuth }, - getModels: input.getModels ?? (async () => models), + getModels: input.getModels ?? (() => models), + refreshModels: input.refreshModels, stream: (model, _context, options) => respond(model, options as StreamOptions | undefined), streamSimple: (model, _context, options) => respond(model, options as SimpleStreamOptions | undefined), }; @@ -127,14 +129,14 @@ describe("Models runtime", () => { models.setProvider(testProvider({ id: "p1", models: [testModel("p1", "m1"), testModel("p1", "m2")] })); models.setProvider(testProvider({ id: "p2", models: [testModel("p2", "m3")] })); - expect((await models.getModels()).map((m) => m.id)).toEqual(["m1", "m2", "m3"]); - expect((await models.getModels("p1")).map((m) => m.id)).toEqual(["m1", "m2"]); - expect((await models.getModels("nope")).length).toBe(0); - expect((await models.getModel("p2", "m3"))?.id).toBe("m3"); - expect(await models.getModel("p2", "missing")).toBeUndefined(); + expect(models.getModels().map((m) => m.id)).toEqual(["m1", "m2", "m3"]); + expect(models.getModels("p1").map((m) => m.id)).toEqual(["m1", "m2"]); + expect(models.getModels("nope").length).toBe(0); + expect(models.getModel("p2", "m3")?.id).toBe("m3"); + expect(models.getModel("p2", "missing")).toBeUndefined(); // hasApi() narrows dynamically looked-up models with a runtime check - const found = await models.getModel("p2", "m3"); + const found = models.getModel("p2", "m3"); expect(found && hasApi(found, "openai-completions")).toBe(false); expect(found && hasApi(found, "test-api")).toBe(true); if (found && hasApi(found, "test-api")) { @@ -143,48 +145,63 @@ describe("Models runtime", () => { } }); - it("swallows provider source failures for both all-provider and single-provider listing", async () => { + it("swallows provider source failures for both all-provider and single-provider listing", () => { const models = createModels(); models.setProvider( testProvider({ id: "broken", - getModels: async () => { + getModels: () => { throw new Error("boom"); }, }), ); models.setProvider(testProvider({ id: "ok", models: [testModel("ok", "m1")] })); - expect((await models.getModels()).map((m) => m.id)).toEqual(["m1"]); - expect(await models.getModels("broken")).toEqual([]); + expect(models.getModels().map((m) => m.id)).toEqual(["m1"]); + expect(models.getModels("broken")).toEqual([]); // precise failures come from the provider directly - await expect(models.getProvider("broken")?.getModels()).rejects.toThrow("boom"); - - // even sync-throwing (non-async) provider implementations are isolated - models.setProvider({ - ...testProvider({ id: "sync-broken" }), - getModels: () => { - throw new Error("sync boom"); - }, - }); - expect((await models.getModels()).map((m) => m.id)).toEqual(["m1"]); + expect(() => models.getProvider("broken")?.getModels()).toThrow("boom"); }); - it("supports getModels(options) without a provider id", async () => { - const seen: ({ forceRefresh?: boolean } | undefined)[] = []; + it("refresh() updates dynamic providers; single-provider refresh failures reject", async () => { + let list = [testModel("dyn", "before")]; + let refreshes = 0; const models = createModels(); - models.setProvider(testProvider({ id: "p1", models: [testModel("p1", "m1")] })); - models.setProvider({ - ...testProvider({ id: "p2" }), - getModels: async (options) => { - seen.push(options); - return [testModel("p2", "m2")]; - }, - }); + models.setProvider( + testProvider({ + id: "dyn", + getModels: () => list, + refreshModels: async () => { + refreshes++; + list = [testModel("dyn", "after")]; + }, + }), + ); + models.setProvider(testProvider({ id: "static", models: [testModel("static", "s1")] })); - const all = await models.getModels({ forceRefresh: true }); - expect(all.map((m) => m.id)).toEqual(["m1", "m2"]); - expect(seen).toEqual([{ forceRefresh: true }]); + expect(models.getModel("dyn", "before")).toBeDefined(); + await models.refresh("dyn"); + expect(refreshes).toBe(1); + expect(models.getModel("dyn", "after")).toBeDefined(); + expect(models.getModel("dyn", "before")).toBeUndefined(); + + // static providers are no-ops; refresh-all is best-effort + await models.refresh("static"); + await models.refresh(); + expect(refreshes).toBe(2); + + // single-provider refresh failures reject with ModelsError + models.setProvider( + testProvider({ + id: "flaky", + refreshModels: async () => { + throw new Error("fetch failed"); + }, + }), + ); + await expect(models.refresh("flaky")).rejects.toMatchObject({ code: "model_source" }); + // refresh-all swallows the same failure + await expect(models.refresh()).resolves.toBeUndefined(); }); it("resolves auth: stored credential owns the provider, ambient only when nothing stored", async () => { diff --git a/packages/ai/test/oauth-auth.test.ts b/packages/ai/test/oauth-auth.test.ts index 579fdb37d..c008b18c0 100644 --- a/packages/ai/test/oauth-auth.test.ts +++ b/packages/ai/test/oauth-auth.test.ts @@ -99,7 +99,7 @@ describe("OAuth through Models.getAuth (lazy load chain)", () => { const models = createModels({ credentials }); models.setProvider(anthropicProvider()); - const model = (await models.getModels("anthropic"))[0]; + const model = models.getModels("anthropic")[0]; const result = await models.getAuth(model); expect(result?.auth.apiKey).toBe("oauth-access-token"); expect(result?.source).toBe("OAuth"); @@ -117,7 +117,7 @@ describe("OAuth through Models.getAuth (lazy load chain)", () => { const models = createModels({ credentials }); models.setProvider(githubCopilotProvider()); - const model = (await models.getModels("github-copilot"))[0]; + const model = models.getModels("github-copilot")[0]; const result = await models.getAuth(model); expect(result?.auth.apiKey).toBe(access); expect(result?.auth.baseUrl).toBe("https://api.business.githubcopilot.com"); diff --git a/packages/ai/test/providers.test.ts b/packages/ai/test/providers.test.ts index 2b41ba1c1..3e6320a98 100644 --- a/packages/ai/test/providers.test.ts +++ b/packages/ai/test/providers.test.ts @@ -26,15 +26,15 @@ describe("builtin providers", () => { expect(providers.length).toBe(builtinProviders().length); expect(providers.map((p) => p.id)).toContain("anthropic"); - const anthropic = await models.getModel("anthropic", "claude-haiku-4-5"); + const anthropic = models.getModel("anthropic", "claude-haiku-4-5"); expect(anthropic?.api).toBe("anthropic-messages"); - const all = await models.getModels(); + const all = models.getModels(); expect(all.length).toBeGreaterThan(500); // every provider lists at least one model and owns its models for (const provider of providers) { - const list = await models.getModels(provider.id); + const list = models.getModels(provider.id); expect(list.length).toBeGreaterThan(0); expect(list.every((m) => m.provider === provider.id)).toBe(true); } @@ -45,7 +45,7 @@ describe("builtin providers", () => { authContext: fakeAuthContext({ ANTHROPIC_API_KEY: "key", ANTHROPIC_OAUTH_TOKEN: "oauth-token" }), }); models.setProvider(anthropicProvider()); - const model = (await models.getModel("anthropic", "claude-haiku-4-5"))!; + const model = models.getModel("anthropic", "claude-haiku-4-5")!; const result = await models.getAuth(model); expect(result?.auth.apiKey).toBe("oauth-token"); @@ -55,7 +55,7 @@ describe("builtin providers", () => { it("reports bedrock as configured from ambient AWS credentials without an api key", async () => { const models = createModels({ authContext: fakeAuthContext({ AWS_PROFILE: "dev" }) }); models.setProvider(amazonBedrockProvider()); - const model = (await models.getModels("amazon-bedrock"))[0]; + const model = models.getModels("amazon-bedrock")[0]; const result = await models.getAuth(model); expect(result?.auth).toEqual({}); @@ -72,7 +72,7 @@ describe("builtin providers", () => { authContext: fakeAuthContext({ GOOGLE_CLOUD_PROJECT: "proj", GOOGLE_CLOUD_LOCATION: "us-central1" }, [adc]), }); configured.setProvider(googleVertexProvider()); - const model = (await configured.getModels("google-vertex"))[0]; + const model = configured.getModels("google-vertex")[0]; const result = await configured.getAuth(model); expect(result?.auth).toEqual({}); @@ -180,15 +180,28 @@ describe("createProvider", () => { expect(result.errorMessage).toContain("no API implementation"); }); - it("supports async model listers", async () => { + it("supports dynamic providers: empty until refreshed, in-flight refreshes deduped", async () => { + let fetches = 0; const provider = createProvider({ id: "dynamic", auth: { apiKey: { name: "Test", resolve: async () => ({ auth: {} }) } }, - models: async () => [testModel("api-a", "listed")], + models: [], + refreshModels: async () => { + fetches++; + await new Promise((resolve) => setTimeout(resolve, 5)); + return [testModel("api-a", "listed")]; + }, api: recordingStreams("a", []), }); - const models = await provider.getModels(); - expect(models.map((m) => m.id)).toEqual(["listed"]); + + expect(provider.getModels()).toEqual([]); + await Promise.all([provider.refreshModels?.(), provider.refreshModels?.()]); + expect(fetches).toBe(1); + expect(provider.getModels().map((m) => m.id)).toEqual(["listed"]); + + // a later refresh fetches again + await provider.refreshModels?.(); + expect(fetches).toBe(2); }); }); @@ -199,7 +212,7 @@ describe("fauxProvider", () => { models.setProvider(faux.provider); faux.setResponses([fauxAssistantMessage("hello from faux")]); - const model = (await models.getModels(faux.provider.id))[0]; + const model = models.getModels(faux.provider.id)[0]; const result = await models.completeSimple(model, context); expect(result.stopReason).toBe("stop"); expect(result.content).toEqual([{ type: "text", text: "hello from faux" }]); diff --git a/packages/ai/test/scratch.ts b/packages/ai/test/scratch.ts index 5e470cbc4..c2d836494 100644 --- a/packages/ai/test/scratch.ts +++ b/packages/ai/test/scratch.ts @@ -18,7 +18,7 @@ models.setProvider(anthropicProvider()); // 2. Look up a model and check auth. // --------------------------------------------------------------------------- -const model = await models.getModel("anthropic", "claude-haiku-4-5"); +const model = models.getModel("anthropic", "claude-haiku-4-5"); if (!model) throw new Error("model not found"); const auth = await models.getAuth(model); diff --git a/packages/coding-agent/src/core/auth-storage.ts b/packages/coding-agent/src/core/auth-storage.ts index 6b9f2f3a4..b30d78327 100644 --- a/packages/coding-agent/src/core/auth-storage.ts +++ b/packages/coding-agent/src/core/auth-storage.ts @@ -198,7 +198,6 @@ export class InMemoryAuthStorageBackend implements AuthStorageBackend { export class AuthStorage { private data: AuthStorageData = {}; private runtimeOverrides: Map = new Map(); - private fallbackResolver?: (provider: string) => string | undefined; private loadError: Error | null = null; private errors: Error[] = []; private storage: AuthStorageBackend; @@ -237,14 +236,6 @@ export class AuthStorage { this.runtimeOverrides.delete(provider); } - /** - * Set a fallback resolver for API keys not found in auth.json or env vars. - * Used for custom provider keys from models.json. - */ - setFallbackResolver(resolver: (provider: string) => string | undefined): void { - this.fallbackResolver = resolver; - } - private recordError(error: unknown): void { const normalizedError = error instanceof Error ? error : new Error(String(error)); this.errors.push(normalizedError); @@ -341,7 +332,6 @@ export class AuthStorage { if (this.runtimeOverrides.has(provider)) return true; if (this.data[provider]) return true; if (getEnvApiKey(provider)) return true; - if (this.fallbackResolver?.(provider)) return true; return false; } @@ -362,10 +352,6 @@ export class AuthStorage { return { configured: false, source: "environment", label: envKeys[0] }; } - if (this.fallbackResolver?.(provider)) { - return { configured: false, source: "fallback", label: "custom provider config" }; - } - return { configured: false }; } @@ -459,9 +445,8 @@ export class AuthStorage { * 2. API key from auth.json * 3. OAuth token from auth.json (auto-refreshed with locking) * 4. Environment variable - * 5. Fallback resolver (models.json custom providers) */ - async getApiKey(providerId: string, options?: { includeFallback?: boolean }): Promise { + async getApiKey(providerId: string): Promise { // Runtime override takes highest priority const runtimeKey = this.runtimeOverrides.get(providerId); if (runtimeKey) { @@ -516,11 +501,6 @@ export class AuthStorage { const envKey = getEnvApiKey(providerId); if (envKey) return envKey; - // Fall back to custom resolver (e.g., models.json custom providers) - if (options?.includeFallback !== false) { - return this.fallbackResolver?.(providerId) ?? undefined; - } - return undefined; } diff --git a/packages/coding-agent/src/core/model-registry.ts b/packages/coding-agent/src/core/model-registry.ts index 114cbb004..39246d18a 100644 --- a/packages/coding-agent/src/core/model-registry.ts +++ b/packages/coding-agent/src/core/model-registry.ts @@ -757,7 +757,7 @@ export class ModelRegistry { async getApiKeyAndHeaders(model: Model): Promise { try { const providerConfig = this.providerRequestConfigs.get(model.provider); - const apiKeyFromAuthStorage = await this.authStorage.getApiKey(model.provider, { includeFallback: false }); + const apiKeyFromAuthStorage = await this.authStorage.getApiKey(model.provider); const apiKey = apiKeyFromAuthStorage ?? (providerConfig?.apiKey @@ -844,7 +844,7 @@ export class ModelRegistry { * Get API key for a provider. */ async getApiKeyForProvider(provider: string): Promise { - const apiKey = await this.authStorage.getApiKey(provider, { includeFallback: false }); + const apiKey = await this.authStorage.getApiKey(provider); if (apiKey !== undefined) { return apiKey; }