fix: prefer configured provider when default provider is removed from config#38947
fix: prefer configured provider when default provider is removed from config#38947davidemanuelDEV wants to merge 1 commit intoopenclaw:mainfrom
Conversation
… config When no explicit model is set in agents.defaults.model, resolveConfiguredModelRef falls back to the hardcoded default (anthropic/claude-opus-4-6). If the user has removed anthropic from models.providers and configured a different provider, status commands still report the stale anthropic model as the default. Now, before returning the hardcoded fallback, the function checks whether the default provider is present in cfg.models.providers. If it is not but another provider with models is configured, it returns that provider's first model instead. This ensures openclaw status and openclaw models status --json report the actual effective default model, not a stale reference to a removed provider. Fixes openclaw#38880
Greptile SummaryThis PR adds a guard in The approach is reasonable, but two normalization gaps in the new code can produce incorrect behavior:
Three new tests cover the happy-path scenarios but do not exercise provider-ID normalization edge cases, so these regressions are not caught by the test suite. Confidence Score: 3/5
Last reviewed commit: 92a93b3 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // from a removed provider. (See #38880) | ||
| const configuredProviders = params.cfg.models?.providers; | ||
| if (configuredProviders && typeof configuredProviders === "object") { | ||
| const hasDefaultProvider = Boolean(configuredProviders[params.defaultProvider]); |
There was a problem hiding this comment.
Provider lookup bypasses normalization
configuredProviders[params.defaultProvider] is a raw object-key lookup, but provider IDs throughout this codebase are normalized via normalizeProviderId before comparison. For example, a user who writes "z.ai" in their config has that stored as-is, yet the canonical ID is "zai". If the defaultProvider is "anthropic" but the user stored it as "Anthropic" (or any other known alias), this check would return undefined and incorrectly conclude the default provider is absent — then silently switch to a different provider.
The correct check should use the existing findNormalizedProviderValue (or findNormalizedProviderKey) utility that already handles this:
| const hasDefaultProvider = Boolean(configuredProviders[params.defaultProvider]); | |
| const hasDefaultProvider = Boolean(findNormalizedProviderValue(configuredProviders, params.defaultProvider)); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/model-selection.ts
Line: 326
Comment:
**Provider lookup bypasses normalization**
`configuredProviders[params.defaultProvider]` is a raw object-key lookup, but provider IDs throughout this codebase are normalized via `normalizeProviderId` before comparison. For example, a user who writes `"z.ai"` in their config has that stored as-is, yet the canonical ID is `"zai"`. If the `defaultProvider` is `"anthropic"` but the user stored it as `"Anthropic"` (or any other known alias), this check would return `undefined` and incorrectly conclude the default provider is absent — then silently switch to a different provider.
The correct check should use the existing `findNormalizedProviderValue` (or `findNormalizedProviderKey`) utility that already handles this:
```suggestion
const hasDefaultProvider = Boolean(findNormalizedProviderValue(configuredProviders, params.defaultProvider));
```
How can I resolve this? If you propose a fix, please make it concise.| if (availableProvider) { | ||
| const [providerName, providerCfg] = availableProvider; | ||
| const firstModel = providerCfg.models[0]; | ||
| return { provider: providerName, model: firstModel.id }; |
There was a problem hiding this comment.
Returned provider name is not normalized
providerName is the raw key from the config object (e.g. "z.ai", "Anthropic", "bytedance"), but every other return path in resolveConfiguredModelRef returns a normalized provider ID (via normalizeModelRef → normalizeProviderId). Returning a non-normalized ID here can cause subtle mismatches downstream when callers compare the provider string against catalog entries, auth lookups, or allowlist checks.
The provider name should be normalized before returning:
| if (availableProvider) { | |
| const [providerName, providerCfg] = availableProvider; | |
| const firstModel = providerCfg.models[0]; | |
| return { provider: providerName, model: firstModel.id }; | |
| return { provider: normalizeProviderId(providerName), model: firstModel.id }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/model-selection.ts
Line: 335-338
Comment:
**Returned provider name is not normalized**
`providerName` is the raw key from the config object (e.g. `"z.ai"`, `"Anthropic"`, `"bytedance"`), but every other return path in `resolveConfiguredModelRef` returns a normalized provider ID (via `normalizeModelRef` → `normalizeProviderId`). Returning a non-normalized ID here can cause subtle mismatches downstream when callers compare the provider string against catalog entries, auth lookups, or allowlist checks.
The provider name should be normalized before returning:
```suggestion
return { provider: normalizeProviderId(providerName), model: firstModel.id };
```
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92a93b3796
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const hasDefaultProvider = Boolean(configuredProviders[params.defaultProvider]); | ||
| if (!hasDefaultProvider) { |
There was a problem hiding this comment.
This fallback path treats cfg.models.providers as the full provider set, but in OpenClaw models.mode defaults to merge (src/agents/models-config.ts:17,222), where built-in/implicit providers can still exist even if they are not listed under cfg.models.providers. With this check, any user who adds a custom provider in merge mode and leaves agents.defaults.model unset will now silently switch the effective default to that custom provider’s first model, changing runtime defaults and status output unexpectedly; the fallback should only trigger when the effective catalog has actually dropped the default provider (for example, replace mode).
Useful? React with 👍 / 👎.
mdlmarkham
left a comment
mdlmarkham
left a comment
There was a problem hiding this comment.
Review: Prefer Configured Provider When Default Removed ✅
Verdict: Clean fix with good tests. Ready to merge.
Problem
When users:
- Configure a custom provider (e.g.,
n1n) - Remove the default provider (
anthropic) from config - Don't set an explicit model in
agents.defaults.model
The status commands still show anthropic/claude-opus-4-6 as default — even though it's unavailable.
Impact:
openclaw statusshows wrong modelopenclaw models status --jsonreports stale default- Runtime correctly uses configured provider, but CLI status is misleading
The Fix
// Before: always return hardcoded default
return { provider: defaultProvider, model: defaultModel };
// After: check if default provider exists
if (cfg.models.providers[defaultProvider]) {
return { provider: defaultProvider, model: defaultModel };
}
// If not, find first configured provider with models
const configuredProvider = Object.keys(cfg.models.providers).find(/* has models */);
return { provider: configuredProvider, model: configuredProvider.models[0].id };Test Coverage
- ✅ Provider removed → falls back to first configured provider
- ✅ Default provider exists → uses default (no regression)
- ✅ No providers with models → falls back to hardcoded default
- ✅ Empty provider → skipped (no models)
Edge Cases
-
Multiple configured providers: Returns first alphabetical provider with models. This is deterministic but arbitrary — operators should set explicit model if order matters.
-
All providers empty: Falls back to hardcoded default (
anthropic/claude-opus-4-6). This is correct — better to show unavailable default than crash. -
Provider added after removal: Next status reflects the new provider correctly.
Minor Suggestion
Consider logging when falling back to configured provider vs hardcoded default:
log.debug(`Default provider ${defaultProvider} not in config, using ${configuredProvider}`);This helps debugging if someone wonders why their status shows a different model than expected.
Recommendation: Approve. Fixes misleading status display, minimal change surface, good tests.
Co-authored-by: davidemanuelDEV <[email protected]>
|
Landed. Thank you @davidemanuelDEV. What I did:
SHA hashes:
Thanks again for the fix. |
* main: (133 commits) reduce image size, offer slim image (openclaw#38479) fix(security): harden install base drift cleanup fix(agents): respect explicit provider baseUrl in merge mode (openclaw#39103) fix(agents): apply contextTokens cap for compaction threshold (openclaw#39099) fix(exec): block dangerous override-only env pivots fix(security): stage installs before publish fix(daemon): normalise whitespace in checkTokenDrift to prevent false-positive warning (openclaw#39108) fix(security): harden fs-safe copy writes refactor: dedupe bluebubbles webhook auth test setup refactor: dedupe discord native command test scaffolding refactor: dedupe anthropic probe target test setup refactor: dedupe minimax provider auth test setup refactor: dedupe runtime snapshot test fixtures fix: harden zip extraction writes fix(tests): stabilize diffs localReq headers (supersedes openclaw#39063) fix: harden workspace skill path containment fix(agents): land openclaw#38935 from @MumuTW fix(models): land openclaw#38947 from @davidemanuelDEV fix(gateway): land openclaw#39064 from @Narcooo fix(models-auth): land openclaw#38951 from @MumuTW ...
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
Co-authored-by: davidemanuelDEV <[email protected]>
(cherry picked from commit 231c1fa) Upstream files discarded (fork-deleted): - src/agents/model-selection.test.ts - src/agents/model-selection.ts
(cherry picked from commit 231c1fa) Upstream files discarded (fork-deleted): - src/agents/model-selection.test.ts - src/agents/model-selection.ts
4 participants
Summary
When no explicit model is set in
agents.defaults.model,resolveConfiguredModelReffalls back to the hardcoded default (anthropic/claude-opus-4-6). If the user has removed anthropic frommodels.providersand configured a different provider,openclaw statusandopenclaw models status --jsonstill report the stale anthropic model as the default — even though runtime sessions correctly use the configured provider.Root Cause
resolveConfiguredModelRef()unconditionally returns{ provider: defaultProvider, model: defaultModel }when no explicit model is configured, without checking whether the default provider is actually available.Fix
Before returning the hardcoded fallback, the function now checks whether the default provider exists in
cfg.models.providers. If it does not, but another provider with models is configured, it returns that provider's first model instead.This ensures:
openclaw statusshows the correct effective default modelopenclaw models status --jsonreports accuratedefaultModelandresolvedDefaultmissingProvidersInUseno longer falsely flags removed providersTests
Added 3 new test cases to
model-selection.test.ts:models.providersmodels.providersAll 43 tests in
model-selection.test.tspass.Fixes #38880