Repository navigation
fix(core): do not run on another provider when the configured one is missing - #59238
2SinceJuly wants to merge 3 commits into
Conversation
…missing A `model` that names a provider whose config entry the schema rejected made `Model.default` fall back to the first available model, so prompts and tool calls silently ran on the built-in free provider. Resolve nothing in that case, and stop the resolver from picking an arbitrary second fallback. A configured model on a provider that still exists keeps its existing fallback.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works: with the config from #58909, the run on the base goes to opencode/big-pickle and sends the request to the OpenCode endpoint. With this PR it stops with No model is available for session … and nothing goes to another provider. The explicit --model path is unchanged. I checked it with the issue's reproduction script, run from source on both checkouts. The catalog, model-free, model-resolver and session-runner tests in packages/core pass (317), and typecheck:tsc passes.
Public API: unchanged. The default-model HTTP route and the plugin models.default() can now return nothing when the configured provider is missing.
Concerns:
- The resolver change removes the package fallback for users with no
modelconfigured too, not just for a configured default (see the inline comment). - The new error doesn't say which model or provider could not be found, unlike
Model unavailable: probe/probe-1on the--modelpath. Namingprobe/probe-1would make the dropped config entry much easier to find. This can be a follow-up. - Side effect worth confirming: a default on a provider removed by managed policy (
provider.usedeny) also gives "no model" now, instead of falling back to an allowed provider. This is probably the right behaviour, but please confirm it is intended.
| ), | ||
| ), | ||
| ) | ||
| : yield* models.default().pipe(Effect.map((model) => (model && hasPackage(model) ? model : undefined))) |
There was a problem hiding this comment.
This removes the second fallback for every case, not only when a default model is configured. When no model is configured, Model.default() returns available.find(supportsText). If that model has no package (hasPackage false), the resolver used to pick the next text model that has one; now the session gets no model at all. Only the configured-default case needs to stop falling back. Could you keep the available().find(hasPackage && supportsText) fallback when no default is configured, or move the package check into the unconfigured branch of Model.default?
Only a configured default whose provider was dropped should stop resolving. When no model is configured, the resolver still prefers the first text model that has a package, as before.
|
Thanks — addressed the three points.
Re-ran the catalog, model-free, model-resolver, session-runner, session-runner-recorded, session-title and generate suites in |
There was a problem hiding this comment.
The new commit addresses the earlier point: with no model configured, the resolver again falls back to the first text model that has a package. The issue's reproduction still stops with No model is available for session … instead of running on big-pickle. The catalog, model-free, model-resolver and session-runner tests pass, and so does typecheck:tsc. Public API: unchanged.
One remaining case, in the restored fallback: it also runs when a model is configured but has no package (see the inline comment).
| Effect.flatMap((model) => { | ||
| if (model === undefined) return Effect.succeed(undefined) | ||
| if (hasPackage(model)) return Effect.succeed(model) | ||
| // No model is configured, so prefer the first text model that can actually be loaded. |
There was a problem hiding this comment.
This branch is reached whenever the default model has no package, including when the user configured it. With a valid custom provider that omits package and model: probe/probe-1, the run silently goes to opencode/flint-40b-free. --model probe/probe-1 fails with Unsupported package instead. That's the same silent switch to another provider this PR is fixing. The fallback could apply only when no default model is configured, for example by having Model.default return the configured model and letting load fail on it, or by checking for a configured default here.
Only an unconfigured default may pick another loadable model. A configured default without a package now fails while loading, like `--model` does, instead of running on a different provider.
|
Fixed in d619287. Regression test: with a registered The plugin-facing API is unchanged; this only adds one method to the internal Core Re-ran the catalog, model-free, model-resolver, session-runner, session-runner-recorded, session-title and generate suites: 350 pass on macOS and in the Linux container, |
There was a problem hiding this comment.
Both earlier points are addressed. With no model configured, the resolver again falls back to a text model that has a package. A configured default is now used as it is: a model with no package fails with Unsupported package for probe/probe-1, the same as --model, instead of running on another provider. I checked both cases from source, along with the #58909 reproduction, which still stops with No model is available for session …. The nearby core tests pass (358) and so does typecheck:tsc. Public API: unchanged (Model.configuredDefault is a new method on the internal Model.Service only).
Issue for this PR
Closes #58909
Type of change
What does this PR do?
When
modelnames a provider whose config entry the schema rejected, the provider is dropped from the catalog.Model.defaultthen returned the first available model, so the configured default silently resolved to a different provider — in practice the built-in free one — and the prompt and its tool calls ran there.The default now resolves to nothing when its provider was never registered, and
ModelResolverno longer picks an arbitrary model as a second fallback. A configured model on a provider that still exists keeps the existing fallback, including a disabled provider or a model that later left the catalog.The run now stops with
No model is available for session …instead of using another provider. It does not yet name the dropped provider; making the rejected config entry itself fail would be a larger change.How did you verify your code works?
With the config from the issue (invalid
capabilities.reasoning,model: probe/probe-1) on an isolatedXDG_*setup:> build · big-pickle, then the free-usage error — the prompt went to the built-in provider.Error: No model is available for session …, and no request leaves for another provider.Tests:
Model.defaultandModelResolver.resolvereturn nothing when the configured provider is missing. Both fail ondevand pass with the change.bun testfor the catalog, model-free, model-resolver, session-runner, session-runner-recorded, session-title and generate suites inpackages/core: 349 pass.oven/bun:1.4.4, arm64): 349 pass.bun typecheckinpackages/corereports no type errors.Checklist
Added by the opencode bot (AI-generated).
Related: #55150, #50340, #53744, #54786