fix(provider): keep a root default model with the provider that owns it

Cause: with `provider = "openai"`, `default_text_model = "gpui-fixture"`
and no `[providers.openai].model`, the root alias is the active route's
fallback and nothing records which provider it belongs to.
- TUI: a session-local `/provider <other>` left the alias at the root, so
  `/provider openai` afterwards resolved with `openai != api_provider()`
  and fell through to the catalog default `gpt-5.6`.
- Engine: `reconcile_root_model_aliases` relocated the alias only when the
  incoming route failed validation. A pass-through incoming route
  (openrouter, zai, ...) inherited `gpui-fixture`, and `GET /v1/providers`
  advertised OpenAI's `default_model` as `gpt-5.6`, which is what the
  desktop model chip picked for "OpenAI-compatible".

Fix: `Config::root_model_alias_owned_by_outgoing` names the outgoing
route and value when that route has no leaf and was actually resolving
the alias. The persisted writer moves it onto the outgoing leaf and
clears the root; the TUI switch applies the same move in memory. The
resolution order is then providers.<p>.model, the root alias while its
own provider is active, then the catalog default. Aliases the outgoing
route never resolved (shadowed by a leaf, `auto`, foreign ids dropped by
`default_model`) keep their previous handling. Two persistence tests that
asserted the alias stays at the root in the no-leaf case now assert it
moves to the outgoing leaf; switching back is still checked.

Tests (focused, CARGO_BUILD_JOBS=4):
- new: tui::ui::tests::provider_switch_back_lands_on_root_default_owned_by_that_provider
  and runtime_api::tests::switch_provider_away_and_back_keeps_the_root_default_with_its_route
  fail without the fix (left: "gpt-5.6" right: "gpui-fixture"; openrouter
  body model "gpui-fixture"):
  test result: FAILED. 3 passed; 2 failed; 0 ignored; 0 measured; 13796 filtered out
- with the fix, filter root_default:
  test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 13796 filtered out
- filters config_persistence default_text_model alias route_save save_route
  provider_picker route_preferences provider_switch switch_provider root_default:
  test result: ok. 469 passed; 0 failed; 0 ignored; 0 measured; 13332 filtered out
- filters config::tests model_inventory route_runtime runtime_api::tests::switch
  runtime_api::tests::get_config provider:
  test result: ok. 1302 passed; 0 failed; 1 ignored; 0 measured; 12498 filtered out

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
This commit is contained in:
CodeWhale BotandClaude Opus 5.5 committed 2026-09-27 20:09:30 -07:00
1 parent e4c94c8495
commit 1394c299e7
8 files changed
+238 -21

No files matched your search

+8
View File
@@ -29,6 +29,14 @@ quieter, and Fleet runs can be checked before they spend anything.
no-follow reads and writes. Compaction loads pinned anchors only from
trusted workspaces. Validate registry skill names before selecting cache paths.
### Fixed
- Switching providers keeps a model set only in the root `default_text_model`
with the provider it belongs to. Switching away and back (`/provider` in the
TUI, or the desktop app's model chip) used to land on the provider's catalog
default such as `gpt-5.6`, and a pass-through provider switched to in between
inherited the other provider's model.
### Contributors
- **[@gaord](https://github.com/gaord)** — let a client fork a thread at a named turn ([#6580](https://github.com/Hmbown/Codewhale/pull/6580)), let undo roll back files for the turn it is undoing ([#6483](https://github.com/Hmbown/Codewhale/pull/6483)), stopped resume and fork from duplicating threads and sessions ([#6406](https://github.com/Hmbown/Codewhale/pull/6406)), and exposed user-defined provider routes to native clients ([#6404](https://github.com/Hmbown/Codewhale/pull/6404)).
+8
View File
@@ -29,6 +29,14 @@ quieter, and Fleet runs can be checked before they spend anything.
no-follow reads and writes. Compaction loads pinned anchors only from
trusted workspaces. Validate registry skill names before selecting cache paths.
### Fixed
- Switching providers keeps a model set only in the root `default_text_model`
with the provider it belongs to. Switching away and back (`/provider` in the
TUI, or the desktop app's model chip) used to land on the provider's catalog
default such as `gpt-5.6`, and a pass-through provider switched to in between
inherited the other provider's model.
### Contributors
- **[@gaord](https://github.com/gaord)** — let a client fork a thread at a named turn ([#6580](https://github.com/Hmbown/Codewhale/pull/6580)), let undo roll back files for the turn it is undoing ([#6483](https://github.com/Hmbown/Codewhale/pull/6483)), stopped resume and fork from duplicating threads and sessions ([#6406](https://github.com/Hmbown/Codewhale/pull/6406)), and exposed user-defined provider routes to native clients ([#6404](https://github.com/Hmbown/Codewhale/pull/6404)).
+37
View File
@@ -6243,6 +6243,43 @@ impl Config {
deepseek_alias_deprecation(&alias)
}
/// The root `default_text_model` alias is the *active* route's fallback.
/// When a switch to `incoming` leaves a route that has no model leaf of its
/// own and was actually resolving the alias, return that outgoing identity
/// and value: the choice belongs to the outgoing route and must follow it
/// onto its own leaf, rather than stay at the root where the incoming route
/// would inherit it and the outgoing route would forget it (falling back to
/// its catalog default on the way back).
pub(crate) fn root_model_alias_owned_by_outgoing(
&self,
incoming: &ProviderIdentity,
) -> Option<(ProviderIdentity, String)> {
let value = self.default_text_model.as_deref()?.trim();
if value.is_empty()
|| value.eq_ignore_ascii_case("auto")
|| value.chars().any(char::is_control)
{
return None;
}
let outgoing = self.active_provider_identity(self.api_provider()).ok()?;
// An unnamed custom route stores its model in the alias itself.
if outgoing == *incoming
|| (outgoing.provider == ApiProvider::Custom && outgoing.persisted_id().is_none())
{
return None;
}
let mut scoped = self.clone();
scoped.scope_to_provider_identity(&outgoing);
if scoped
.provider_config_for(outgoing.provider)
.and_then(|entry| entry.model.as_deref())
.is_some()
{
return None;
}
(scoped.default_model() == value).then(|| (outgoing, value.to_string()))
}
#[must_use]
pub fn default_model(&self) -> String {
if self.default_text_model.is_none() && self.legacy_model.is_some() {
+41 -14
View File
@@ -248,13 +248,16 @@ pub(crate) fn persist_provider_selection(
/// when the selected route has no model of its own. A switch that leaves the
/// incoming route without a leaf therefore hands it an alias naming the route
/// on its way out, and `Config::load` rejects the file the caller just wrote —
/// a committed switch that produces an unloadable config.
/// a committed switch that produces an unloadable config — or, on a
/// pass-through incoming route that loads it, a model the incoming route never
/// chose while the outgoing route comes back on its catalog default.
///
/// Relocate that value onto the outgoing route's own canonical leaf, because
/// it is real saved state, and only then clear the alias. An alias the
/// incoming route already shadows with its own leaf is inert and stays:
/// deleting a saved choice to satisfy validation of a value nothing resolves
/// is data loss, not a repair. Likewise an unnamed custom route stores its
/// When the outgoing route has no leaf and was resolving the alias, relocate
/// that value onto the outgoing route's own canonical leaf, because it is real
/// saved state, and only then clear the alias. Any other alias the incoming
/// route already shadows with its own leaf is inert and stays: deleting a
/// saved choice to satisfy validation of a value nothing resolves is data
/// loss, not a repair. Likewise an unnamed custom route stores its
/// model in the alias itself and has no leaf to receive it. If the incoming
/// route cannot shadow that value, refuse the switch atomically and ask for an
/// explicit destination model instead of saving an unloadable configuration.
@@ -273,6 +276,20 @@ pub(crate) fn reconcile_root_model_aliases(
if incoming.provider == ApiProvider::Custom && incoming.persisted_id().is_none() {
return Ok(());
}
// The outgoing route was resolving the alias as its own model. Move it
// onto that route's leaf even when the incoming route could load it: a
// pass-through incoming route would otherwise inherit a model it never
// chose, and the outgoing route would come back on its catalog default.
if let Some((outgoing, value)) = previous.root_model_alias_owned_by_outgoing(incoming) {
set_provider_model_document(
doc,
outgoing.provider,
outgoing.persisted_id().unwrap_or(&outgoing.key),
&value,
)?;
unset_document_value(doc, &["default_text_model"])?;
return Ok(());
}
// Only `default_text_model` is read here. The legacy root `model` key is
// never what blocks a load: `Config::default_model` already refuses to
// route a foreign legacy value to a provider that cannot serve it, and
@@ -1847,14 +1864,24 @@ slot = 1
)
.unwrap();
let doc: toml::Value = toml::from_str(&fs::read_to_string(&path).unwrap()).unwrap();
// The incoming route owns its own leaf, so the outgoing root
// alias is inert. It is a saved choice: deleting it to satisfy
// validation of a value nothing resolves would be data loss.
assert_eq!(
doc[root_key].as_str(),
Some("GLM-4.6"),
"an inert root fallback must survive the switch"
);
// The outgoing route was resolving the root alias only when it
// had no leaf; then the choice moves onto that leaf. Otherwise
// the alias is inert saved state and stays. Neither case may
// delete it.
if root_key == "default_text_model" && existing.is_none() {
assert!(doc.get(root_key).is_none(), "{doc:?}");
assert_eq!(
doc["providers"]["zai"]["model"].as_str(),
Some("GLM-4.6"),
"the outgoing route's choice must follow it onto its leaf"
);
} else {
assert_eq!(
doc[root_key].as_str(),
Some("GLM-4.6"),
"an inert root fallback must survive the switch"
);
}
assert_eq!(
doc["providers"]["deepseek"]["model"].as_str(),
Some("deepseek-v4-pro")
+6 -7
View File
@@ -576,18 +576,17 @@ model = "Other-X"
.collect();
let path = home.path().join("config.toml");
// The incoming route owns its own leaf, so the outgoing root fallback
// is inert saved state. A CLI switch must not delete it, and switching
// back must still find it.
// The outgoing route was resolving the root fallback as its own
// model, so a CLI switch moves it onto that route's leaf (never
// deletes it), and switching back must still find it.
std::fs::write(
&path,
"route_preferences_version = 1\nprovider = \"zai\"\ndefault_text_model = \"GLM-4.6\"\n[providers.deepseek]\nmodel = \"deepseek-v4-pro\"\n",
)?;
set(&path, "provider", "deepseek")?;
assert_eq!(
document(&path)["default_text_model"].as_str(),
Some("GLM-4.6")
);
let doc = document(&path);
assert!(doc.get("default_text_model").is_none());
assert_eq!(doc["providers"]["zai"]["model"].as_str(), Some("GLM-4.6"));
let switched = Config::load(Some(path.clone()), None)
.expect("a CLI provider switch must remain loadable");
assert_eq!(switched.api_provider(), ApiProvider::Deepseek);
+72
View File
@@ -23399,3 +23399,75 @@ mod thread_snapshot_ownership {
Ok(())
}
}
#[tokio::test]
async fn switch_provider_away_and_back_keeps_the_root_default_with_its_route() -> Result<()> {
// Device-test regression: `provider = "openai"` with the model only in the
// root `default_text_model`. Switching to a pass-through route used to leave
// the alias at the root, so that route inherited `gpui-fixture`, the
// provider list advertised OpenAI's catalog default (`gpt-5.6`), and the
// desktop model chip landed there.
let root = std::env::temp_dir().join(format!("codewhale-switch-root-alias-{}", Uuid::new_v4()));
fs::create_dir_all(&root)?;
let config_file = root.join("custom-config.toml");
fs::write(
&config_file,
r#"provider = "openai"
default_text_model = "gpui-fixture"
[providers.openai]
api_key = "sk-test"
base_url = "http://127.0.0.1:9/v1"
[providers.openrouter]
api_key = "sk-or-test"
"#,
)?;
let Some((addr, _runtime_threads, handle)) =
spawn_test_server_with_config_path(config_file.clone()).await?
else {
return Ok(());
};
let client = crate::tls::reqwest_client();
let (status, away) =
post_switch_provider(&client, &addr, "openrouter", &serde_json::json!({})).await;
assert_eq!(status, StatusCode::OK, "body: {away}");
assert_ne!(
away["model"].as_str(),
Some("gpui-fixture"),
"openrouter must not inherit openai's root default: {away}"
);
let persisted = fs::read_to_string(&config_file)?;
assert!(
!persisted.contains("default_text_model"),
"the alias must move off the root. Actual config:\n{persisted}"
);
let providers: serde_json::Value = client
.get(format!("http://{addr}/v1/providers"))
.send()
.await?
.json()
.await?;
let openai = providers["providers"]
.as_array()
.and_then(|entries| entries.iter().find(|entry| entry["id"] == "openai"))
.cloned()
.unwrap_or_default();
assert_eq!(
openai["default_model"].as_str(),
Some("gpui-fixture"),
"the inactive openai entry must keep its configured model, not the catalog default: {openai}"
);
let (status, back) =
post_switch_provider(&client, &addr, "openai", &serde_json::json!({})).await;
assert_eq!(status, StatusCode::OK, "body: {back}");
assert_eq!(back["model"].as_str(), Some("gpui-fixture"), "body: {back}");
assert_eq!(get_config(&client, &addr).await["model"], "gpui-fixture");
handle.abort();
Ok(())
}
+14
View File
@@ -624,6 +624,20 @@ pub(crate) async fn switch_provider(
previous_api_key_env_only: app.api_key_env_only,
});
// A session-local switch keeps the same ownership rule the persisted
// writers apply (`reconcile_root_model_aliases`): the root alias the
// outgoing route was using moves onto that route's own leaf, so coming
// back lands on it instead of the catalog default. Every failure path
// below restores `previous_config`.
if let Some((outgoing, value)) = config
.active_provider_identity(target)
.ok()
.and_then(|incoming| config.root_model_alias_owned_by_outgoing(&incoming))
{
config.set_provider_model_override(outgoing.provider, Some(value));
config.default_text_model = None;
}
let resolved_route = match resolve_runtime_route(config, target, model_override.as_deref()) {
Ok(route) => route,
Err(reason) => {
+52
View File
@@ -31744,3 +31744,55 @@ fn background_review_failed_shell_receipt_uses_context_neutral_copy() {
"shell · false · échec · code de sortie 2"
);
}
#[tokio::test]
async fn provider_switch_back_lands_on_root_default_owned_by_that_provider() {
// Device-test regression: with the openai model only in the root
// `default_text_model`, a session-local `/provider deepseek` left the alias
// at the root, so `/provider openai` came back on the catalog default.
let _home = SettingsHomeGuard::new();
let mut app = create_test_app();
app.api_provider = ApiProvider::Openai;
app.model = "gpui-fixture".to_string();
let mut engine = mock_engine_handle();
let mut config = Config {
provider: Some("openai".to_string()),
default_text_model: Some("gpui-fixture".to_string()),
providers: Some(ProvidersConfig {
openai: ProviderConfig {
api_key: Some("sk-test".to_string()),
base_url: Some("http://127.0.0.1:9/v1".to_string()),
..Default::default()
},
..Default::default()
}),
..Default::default()
}
.with_legacy_root(Some("deepseek-key".to_string()), None);
assert!(
switch_provider(
&mut app,
&mut engine.handle,
&mut config,
ApiProvider::Deepseek,
None,
)
.await
);
assert_eq!(app.api_provider, ApiProvider::Deepseek);
assert_ne!(app.model, "gpui-fixture");
assert!(
switch_provider(
&mut app,
&mut engine.handle,
&mut config,
ApiProvider::Openai,
None,
)
.await
);
assert_eq!(app.api_provider, ApiProvider::Openai);
assert_eq!(app.model, "gpui-fixture");
}