From 9825e4b2fe69d997dbf533e228445d50095e6ba8 Mon Sep 17 00:00:00 2001 From: Abhinav Date: Sat, 14 Nov 2026 01:18:10 +0000 Subject: [PATCH] Use the provider default RPC in the TUI (#61438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What changed - Save default provider selections through `modelProvider/default/set` instead of a generic configuration write. - Use provider display names in save notices, falling back to the provider ID when the name is blank. Replace the restart hint with “New chats now use {name}.” - Prompt users to update the connected app server when it does not support the default provider RPC. ## Testing Update provider tests and snapshots to verify the dedicated RPC, display-name confirmation, and upgrade guidance. Check that only ambiguous write failures invalidate the model catalog. GitOrigin-RevId: cd383a8b85b5d497e7707691cc0b994110716a09 --- codex-rs/tui/src/app/provider.rs | 6 ++- codex-rs/tui/src/app/provider/rpc.rs | 30 +++++------ codex-rs/tui/src/app/provider/write.rs | 28 ++++++---- codex-rs/tui/src/app/provider_tests.rs | 51 ++++++++++++++----- ...ovider__tests__saved_default_provider.snap | 2 +- 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/codex-rs/tui/src/app/provider.rs b/codex-rs/tui/src/app/provider.rs index 95b71752c4..20bc92f7d1 100644 --- a/codex-rs/tui/src/app/provider.rs +++ b/codex-rs/tui/src/app/provider.rs @@ -287,6 +287,7 @@ fn list( let secondary_action = (can_select && !is_default).then(|| { let backend = catalog.backend; let id = provider.id.clone(); + let name = provider.name.clone(); let dismissed = dismissed.clone(); SelectionSecondaryAction { key: key_hint::plain(KeyCode::Char(' ')), @@ -295,7 +296,10 @@ fn list( tx.send(AppEvent::Provider(Event::Write { backend, dismissed: dismissed.clone(), - write: write::Write::Default(id.clone()), + write: write::Write::Default { + id: id.clone(), + name: name.clone(), + }, })) }), } diff --git a/codex-rs/tui/src/app/provider/rpc.rs b/codex-rs/tui/src/app/provider/rpc.rs index 996fddbe86..73807391b3 100644 --- a/codex-rs/tui/src/app/provider/rpc.rs +++ b/codex-rs/tui/src/app/provider/rpc.rs @@ -3,11 +3,10 @@ use codex_app_server_client::AppServerRequestHandle; use codex_app_server_client::TypedRequestError; use codex_app_server_protocol::ClientRequest; -use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::ConfigWriteResponse; -use codex_app_server_protocol::MergeStrategy; use codex_app_server_protocol::ModelProviderConfigWriteParams; use codex_app_server_protocol::ModelProviderConfigWriteResponse; +use codex_app_server_protocol::ModelProviderDefaultSetParams; use codex_app_server_protocol::ModelProviderListParams; use codex_app_server_protocol::ModelProviderListResponse; use codex_app_server_protocol::ModelProviderSummary; @@ -72,14 +71,10 @@ pub(crate) async fn set_default_provider( id: &str, ) -> Result { handle - .request_typed(ClientRequest::ConfigValueWrite { + .request_typed(ClientRequest::ModelProviderDefaultSet { request_id: RequestId::String(Uuid::new_v4().to_string()), - params: ConfigValueWriteParams { - key_path: "model_provider".into(), - value: id.into(), - merge_strategy: MergeStrategy::Replace, - file_path: None, - expected_version: None, + params: ModelProviderDefaultSetParams { + provider_id: id.into(), }, }) .await @@ -93,14 +88,17 @@ pub(crate) fn override_reason(response: ConfigWriteResponse) -> String { } pub(super) fn request_error(err: TypedRequestError) -> String { - if let TypedRequestError::Server { method, source } = &err - && (source.code == -32601 - || (source.code == -32600 - && source - .message - .starts_with(&format!("Invalid request: unknown variant `{method}`")))) - { + if unsupported_method(&err) { return "Update the connected app server to browse model providers.".into(); } format!("Could not load model providers: {err}") } + +pub(super) fn unsupported_method(err: &TypedRequestError) -> bool { + matches!(err, TypedRequestError::Server { method, source } + if source.code == -32601 + || (source.code == -32600 + && source + .message + .starts_with(&format!("Invalid request: unknown variant `{method}`")))) +} diff --git a/codex-rs/tui/src/app/provider/write.rs b/codex-rs/tui/src/app/provider/write.rs index f10b31449d..c67d83d8d5 100644 --- a/codex-rs/tui/src/app/provider/write.rs +++ b/codex-rs/tui/src/app/provider/write.rs @@ -22,7 +22,10 @@ const SAVING_VIEW: &str = "model_provider_saving"; #[derive(derive_more::Debug, Clone)] pub(crate) enum Write { - Default(String), + Default { + id: String, + name: String, + }, #[debug("Settings(..)")] Settings(Box<(editor::Draft, ModelProviderConfigWriteParams)>), } @@ -55,7 +58,7 @@ impl Write { }; let (view, dismissed) = progress(SAVING_VIEW, "Saving model provider…"); let source = match self { - Self::Default(_) => LIST_VIEW, + Self::Default { .. } => LIST_VIEW, Self::Settings(_) => editor::VIEW_ID, }; if !app @@ -99,10 +102,19 @@ impl Write { rpc::ProviderWrite::UnconfirmedCreate => SaveNotice::UnconfirmedCreate, }) } - Self::Default(id) => { + Self::Default { id, name } => { + let name = if name.trim().is_empty() { + id.as_str() + } else { + name.trim() + }; let response = rpc::set_default_provider(handle, id) .await .map_err(|error| match error { + TypedRequestError::Server { .. } if rpc::unsupported_method(&error) => { + "Update the connected app server to change the default model provider." + .into() + } TypedRequestError::Server { .. } => { format!("Could not save provider: {error}") } @@ -110,19 +122,17 @@ impl Write { | TypedRequestError::Deserialize { .. } => { // The server may have committed the write before its reply was lost. *provider_changed = true; - format!("Could not confirm whether {id} was saved: {error}") + format!("Could not confirm whether {name} was saved: {error}") } })?; match response.status { WriteStatus::Ok => { *provider_changed = true; - Ok(SaveNotice::Saved(format!( - "/new now uses {id}. Restart the app server and TUI to refresh /model." - ))) + Ok(SaveNotice::Saved(format!("New chats now use {name}."))) } WriteStatus::OkOverridden => { let reason = rpc::override_reason(response); - Ok(SaveNotice::Saved(format!("Saved {id}. {reason}"))) + Ok(SaveNotice::Saved(format!("Saved {name}. {reason}"))) } } } @@ -157,7 +167,7 @@ impl Completion { }; if present { let view = match self.write { - Write::Default(_) => SAVING_VIEW, + Write::Default { .. } => SAVING_VIEW, Write::Settings(_) => { app.chat_widget.dismiss_provider_view(SAVING_VIEW); LIST_VIEW diff --git a/codex-rs/tui/src/app/provider_tests.rs b/codex-rs/tui/src/app/provider_tests.rs index 3688badc5f..90ee1e507e 100644 --- a/codex-rs/tui/src/app/provider_tests.rs +++ b/codex-rs/tui/src/app/provider_tests.rs @@ -205,7 +205,11 @@ async fn space_saves_once_retries_and_preserves_the_current_chat() -> Result<()> next_provider_event(&mut events).await, ); // The spawned RPC cannot run until this current-thread test yields. - write::Write::Default("openai".into()).start(&mut app, &server); + write::Write::Default { + id: "openai".into(), + name: "OpenAI".into(), + } + .start(&mut app, &server); assert!(std::iter::from_fn(|| events.try_recv().ok()).any(|event| { matches!(event, AppEvent::InsertHistoryCell(cell) if cell.display_lines(/*width*/ 100)[0] .to_string().contains("A provider change is already saving.")) @@ -275,8 +279,8 @@ async fn space_saves_once_retries_and_preserves_the_current_chat() -> Result<()> _ => None, }) .last() - .expect("restart hint in history"); - insta::assert_snapshot!(cell.display_lines(/*width*/ 160)[0].to_string(), @"• /new now uses openai. Restart the app server and TUI to refresh /model."); + .expect("provider selection in history"); + insta::assert_snapshot!(cell.display_lines(/*width*/ 160)[0].to_string(), @"• New chats now use OpenAI."); server.shutdown().await?; Ok(()) } @@ -297,11 +301,17 @@ async fn only_ambiguous_write_failures_invalidate_the_model_catalog() { }))); let disconnected = Err(std::io::Error::other("connection lost")); let unreadable = Ok(Ok(serde_json::Value::Null)); - for (reply, stale) in [ - (rejected, false), - (overridden, false), - (disconnected, true), - (unreadable, true), + let unknown_method = Ok(Err(JSONRPCErrorError { + code: -32600, + message: "Invalid request: unknown variant `modelProvider/default/set`".into(), + data: None, + })); + for (reply, stale, upgrade) in [ + (rejected, false, false), + (overridden, false, false), + (disconnected, true, false), + (unreadable, true, false), + (unknown_method, false, true), ] { let (mut app, mut events, _ops) = make_test_app_with_channels().await; let (client, mut commands, _server_events) = CloudClient::channel(/*capacity*/ 1); @@ -316,16 +326,31 @@ async fn only_ambiguous_write_failures_invalidate_the_model_catalog() { items: vec![back()], ..Default::default() }); - write::Write::Default("gateway".into()).start(&mut app, &server); + write::Write::Default { + id: "gateway".into(), + name: "Example Gateway".into(), + } + .start(&mut app, &server); let command = commands.recv().await.expect("config write"); + let codex_app_server_client::CloudOperation::Request(request) = command.operation else { + panic!("expected a provider selection request"); + }; + assert!( + matches!(*request, codex_app_server_protocol::ClientRequest::ModelProviderDefaultSet { params, .. } + if params.provider_id == "gateway") + ); command .response .send(reply) .expect("write is awaiting a response"); - assert!(matches!( - next_provider_event(&mut events).await, - Event::Written(_) - )); + let written = next_provider_event(&mut events).await; + assert!(matches!(written, Event::Written(_))); + if upgrade { + handle(&mut app, &mut make_test_tui().unwrap(), &server, written); + assert!(std::iter::from_fn(|| events.try_recv().ok()).any(|event| { + matches!(event, AppEvent::InsertHistoryCell(cell) if cell.display_lines(/*width*/ 120)[0].to_string().contains("Update the connected app server to change the default model provider.")) + })); + } let reconnected = ModelCatalog::new(Vec::new()).for_backend(backend, &mut app.provider_writes); assert_eq!(reconnected.provider_may_have_changed(), stale); diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__provider__tests__saved_default_provider.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__provider__tests__saved_default_provider.snap index c1de0615f1..09c6384dae 100644 --- a/codex-rs/tui/src/app/snapshots/codex_tui__app__provider__tests__saved_default_provider.snap +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__provider__tests__saved_default_provider.snap @@ -10,5 +10,5 @@ expression: "render_bottom_popup(&app.chat_widget, 100)" 2. ○ OpenAI openai 3. Create a custom provider or gateway - /new now uses gateway. Restart the app server and TUI to refresh /model. + New chats now use Example Gateway. enter details · esc back