Repository navigation
fix(core): do not run on another provider when the configured one is missing - #59238
2SinceJuly wants to merge 4 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.
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).
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).
189b5d4 to
a7d6276
Compare
|
@rekram1-node bumping this one since it's been quiet since the 26th. Small change, 4 files. I just checked it against current The bug itself: when a provider entry fails schema validation, Let me know if you'd rather see it scoped differently or split up. |
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