mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Use the provider default RPC in the TUI (#61438)
## 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
This commit is contained in:
@@ -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(),
|
||||
},
|
||||
}))
|
||||
}),
|
||||
}
|
||||
|
||||
@@ -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<ConfigWriteResponse, TypedRequestError> {
|
||||
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}`"))))
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user