From 82e6bacb9c6950d3b56cb153847fa80bdd698053 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Tue, 10 Nov 2026 19:42:04 +0000 Subject: [PATCH] Enable model and reasoning effort selection for Cloud conversations (#60651) ## Why Cloud conversations previously used the model returned on activation without exposing model discovery or supporting model changes from the TUI. ## What changed - Fetch the account-scoped Cloud catalog on demand through `/v2/models`, independently of the submission and attachment queue. - Let users stage a supported model and reasoning effort for the next new turn in the current conversation. Send these settings with the user message and show acceptance after successful admission. - Show a loading picker when the catalog is empty, preserve compatibility with Cloud servers that lack discovery, and discard stale catalog results and selections when the account changes. ## Testing Add coverage for catalog request parameters and queue bypass, deferred discovery and compatibility, model and effort serialization, admission outcomes, account invalidation, and opening the picker after discovery. GitOrigin-RevId: 2cd0e7fe5f760d4da9258eefe9f0dcbcd23f9b07 --- codex-rs/app-server-client/src/cloud.rs | 65 ++++++++- .../src/cloud_backend/mod.rs | 3 +- .../src/cloud_backend/requests.rs | 7 + .../src/cloud_backend/submission_tests.rs | 19 ++- codex-rs/app-server-client/src/cloud_tests.rs | 77 ++++++++++ codex-rs/cloud-client/examples/attach.rs | 2 + codex-rs/cloud-client/src/client_tests.rs | 34 +++++ codex-rs/cloud-client/src/lib.rs | 70 ++++++++- codex-rs/cloud-client/src/turns.rs | 9 +- codex-rs/cloud-client/src/turns_tests.rs | 20 ++- codex-rs/cloud-client/src/turns_wire.rs | 4 + codex-rs/cloud-client/src/types.rs | 4 + codex-rs/tui/src/app/cloud_switch_wait.rs | 2 +- codex-rs/tui/src/app/event_dispatch.rs | 15 +- codex-rs/tui/src/app/model_defaults.rs | 45 ++++++ .../tui/src/app/tests/model_defaults_tests.rs | 135 ++++++++++++------ .../tui/src/app/tests/new_session_tests.rs | 2 +- codex-rs/tui/src/app/user_turn_submission.rs | 8 +- codex-rs/tui/src/app_server_session.rs | 15 +- codex-rs/tui/src/app_server_session/models.rs | 22 ++- .../src/app_server_session/models_tests.rs | 84 +++++++++++ .../reasoning_defaults_tests.rs | 1 + .../user_message_submission.rs | 82 +++++++++-- .../workspace_roots_tests.rs | 1 + codex-rs/tui/src/chatwidget.rs | 2 + codex-rs/tui/src/chatwidget/constructor.rs | 1 + .../tui/src/chatwidget/model_popup_state.rs | 22 ++- codex-rs/tui/src/chatwidget/model_popups.rs | 22 ++- codex-rs/tui/src/chatwidget/session_flow.rs | 3 + codex-rs/tui/src/chatwidget/settings.rs | 2 +- .../chatwidget/tests/model_picker_tests.rs | 20 +++ 31 files changed, 709 insertions(+), 89 deletions(-) create mode 100644 codex-rs/tui/src/app_server_session/models_tests.rs diff --git a/codex-rs/app-server-client/src/cloud.rs b/codex-rs/app-server-client/src/cloud.rs index c0438499da..35fea2b547 100644 --- a/codex-rs/app-server-client/src/cloud.rs +++ b/codex-rs/app-server-client/src/cloud.rs @@ -7,6 +7,7 @@ use crate::TypedRequestError; use codex_app_server_protocol::ClientNotification; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::Result as JsonRpcResult; use codex_app_server_protocol::Turn; @@ -62,6 +63,7 @@ pub struct CloudCommand { #[derive(Clone)] pub struct CloudRequestHandle { commands: mpsc::Sender, + catalog: Option>, } /// Tracks only losses queued for delivery. The worker remains the sole owner @@ -126,13 +128,27 @@ impl CloudClient { Self, mpsc::Receiver, mpsc::UnboundedSender, + ) { + Self::channel_with_catalog(capacity, /*catalog*/ None) + } + + pub(super) fn channel_with_catalog( + capacity: usize, + catalog: Option, + ) -> ( + Self, + mpsc::Receiver, + mpsc::UnboundedSender, ) { let (commands, receiver) = mpsc::channel(capacity.max(1)); let (events, event_receiver) = mpsc::unbounded_channel(); ( Self { stream_losses: CloudStreamLosses::default(), - handle: CloudRequestHandle { commands }, + handle: CloudRequestHandle { + commands, + catalog: catalog.map(Arc::new), + }, events: event_receiver, }, receiver, @@ -191,6 +207,47 @@ impl CloudClient { } impl CloudRequestHandle { + /// Whether a completed model read still belongs to the bound Cloud account. + pub fn model_catalog_is_current(&self) -> bool { + self.catalog + .as_ref() + .is_none_or(|catalog| catalog.model_catalog_is_current()) + } + + async fn model_list( + catalog: &codex_cloud_client::Client, + params: &ModelListParams, + ) -> RequestResult { + if params.model_provider.is_some() { + return Err(JSONRPCErrorError { + code: -32602, + message: "Cloud model discovery uses the server's model provider.".into(), + data: None, + }); + } + let result = async { + let body = catalog + .list_models( + params.cursor.as_deref(), + params.limit, + params.include_hidden, + ) + .await + .map_err(|error| error.to_string())?; + if !catalog.model_catalog_is_current() { + return Err("Cloud model catalog account changed.".into()); + } + serde_json::from_slice(&body) + .map_err(|_| "Invalid Cloud model catalog response.".into()) + } + .await; + result.map_err(|message| JSONRPCErrorError { + code: -32000, + message, + data: None, + }) + } + pub(crate) async fn start_project_thread( &self, request_id: RequestId, @@ -242,6 +299,12 @@ impl CloudRequestHandle { } pub(crate) async fn request(&self, request: ClientRequest) -> io::Result { + // Catalog reads must not occupy the serialized send and attachment lane. + if let (Some(catalog), ClientRequest::ModelList { params, .. }) = (&self.catalog, &request) + { + // Keep the catalog HTTP state out of the shared request future. + return Ok(Box::pin(Self::model_list(catalog, params)).await); + } self.send(CloudOperation::Request(Box::new(request))).await } diff --git a/codex-rs/app-server-client/src/cloud_backend/mod.rs b/codex-rs/app-server-client/src/cloud_backend/mod.rs index d54e701a4d..057a41c64a 100644 --- a/codex-rs/app-server-client/src/cloud_backend/mod.rs +++ b/codex-rs/app-server-client/src/cloud_backend/mod.rs @@ -151,7 +151,8 @@ impl Replies { /// Opens an app-server client backed by the authenticated native Cloud client. fn start_worker(client: cloud::Client, account: protocol::GetAccountResponse) -> AppServerClient { - let (app_client, commands, output) = CloudClient::channel(/*capacity*/ 32); + let (app_client, commands, output) = + CloudClient::channel_with_catalog(/*capacity*/ 32, Some(client.clone())); let mut requests = requests::Requests { client, account, diff --git a/codex-rs/app-server-client/src/cloud_backend/requests.rs b/codex-rs/app-server-client/src/cloud_backend/requests.rs index b1b14c6e63..7c01d5ca1a 100644 --- a/codex-rs/app-server-client/src/cloud_backend/requests.rs +++ b/codex-rs/app-server-client/src/cloud_backend/requests.rs @@ -314,6 +314,11 @@ fn cloud_message( ))?; let input = std::mem::take(&mut params.input); let turn_trigger = params.turn_trigger.take(); + let (model, effort) = if turn_trigger.as_deref() == Some("user") { + (params.model.take(), params.effort.take()) + } else { + (None, None) + }; if params != protocol::TurnStartParams::default() { return Err(PreflightRejection( "Changing cloud settings or sending structured input is not supported yet.", @@ -385,6 +390,8 @@ fn cloud_message( client_user_message_id, input: content, turn_trigger, + model, + effort: effort.map(|effort| effort.to_string()), mode: cloud::SubmissionMode::StartIfIdle, shared_access: false, }) diff --git a/codex-rs/app-server-client/src/cloud_backend/submission_tests.rs b/codex-rs/app-server-client/src/cloud_backend/submission_tests.rs index 8ea876ba0f..6d11c89c5d 100644 --- a/codex-rs/app-server-client/src/cloud_backend/submission_tests.rs +++ b/codex-rs/app-server-client/src/cloud_backend/submission_tests.rs @@ -14,6 +14,9 @@ fn submission(message_id: &str) -> protocol::TurnStartParams { protocol::TurnStartParams { thread_id: "t".into(), client_user_message_id: Some(message_id.into()), + model: Some("chosen-model".into()), + effort: Some(codex_protocol::openai_models::ReasoningEffort::High), + turn_trigger: Some("user".into()), input: vec![protocol::UserInput::Text { text: "hello".into(), text_elements: Vec::new(), @@ -114,7 +117,7 @@ async fn native_submission_uses_one_admission_and_keeps_preflight_guards() -> Re for params in [ protocol::TurnStartParams { - model: Some("other".into()), + service_tier: Some(Some("other".into())), ..submission("invalid") }, protocol::TurnStartParams { @@ -189,12 +192,14 @@ async fn native_submission_uses_one_admission_and_keeps_preflight_guards() -> Re client.shutdown().await?; worker.await?; let calls = server.received_requests().await.unwrap(); - assert!(calls.iter().all(|request| { - request - .body - .windows(url.len()) - .any(|bytes| bytes == url.as_bytes()) - })); + for expected in [url.as_bytes(), b"\x4a\x04high", b"\x62\x0cchosen-model"] { + assert!(calls.iter().all(|request| { + request + .body + .windows(expected.len()) + .any(|bytes| bytes == expected) + })); + } assert_eq!( calls .iter() diff --git a/codex-rs/app-server-client/src/cloud_tests.rs b/codex-rs/app-server-client/src/cloud_tests.rs index bee099ec08..f3a44c4b90 100644 --- a/codex-rs/app-server-client/src/cloud_tests.rs +++ b/codex-rs/app-server-client/src/cloud_tests.rs @@ -102,3 +102,80 @@ async fn cloud_shutdown_times_out_when_owner_does_not_respond() { assert!(events.is_closed()); assert!(commands.recv().await.is_none()); } + +#[tokio::test] +async fn native_model_list_bypasses_the_cloud_command_queue() -> color_eyre::Result<()> { + use codex_app_server_protocol::ModelListResponse; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::header; + use wiremock::matchers::method; + use wiremock::matchers::path; + use wiremock::matchers::query_param; + + let server = MockServer::start().await; + let home = tempfile::tempdir()?; + let config = crate::legacy_core::config::ConfigBuilder::default() + .codex_home(home.path().to_owned()) + .fallback_cwd(Some(home.path().to_owned())) + .without_project_context() + .build() + .await?; + let native = codex_cloud_client::Client::new( + &config.http_client_factory(), + &server.uri(), + codex_cloud_client::Credentials::new("synthetic-bearer", "synthetic-account")?, + )?; + let response = serde_json::json!({ + "data": [{ + "id": "test-model", + "model": "test-model", + "displayName": "Test model", + "description": "Cloud catalog fixture", + "hidden": true, + "supportedReasoningEfforts": [], + "defaultReasoningEffort": "medium", + "isDefault": false + }], + "nextCursor": "next" + }); + Mock::given(method("GET")) + .and(path("/v2/models")) + .and(header("authorization", "Bearer synthetic-bearer")) + .and(header("chatgpt-account-id", "synthetic-account")) + .and(query_param("cursor", "previous")) + .and(query_param("limit", "2")) + .and(query_param("includeHidden", "true")) + .respond_with(ResponseTemplate::new(200).set_body_json(&response)) + .expect(1) + .mount(&server) + .await; + + // Leave the command receiver idle. The catalog must finish without its worker. + let (client, mut commands, _events) = + CloudClient::channel_with_catalog(/*capacity*/ 1, Some(native)); + let handle = AppServerClient::Cloud(client).request_handle(); + let result = tokio::time::timeout( + std::time::Duration::from_secs(/*secs*/ 5), + handle.request_typed::(ClientRequest::ModelList { + request_id: RequestId::Integer(1), + params: ModelListParams { + cursor: Some("previous".into()), + limit: Some(2), + include_hidden: Some(true), + model_provider: None, + }, + }), + ) + .await??; + assert_eq!( + result, + serde_json::from_value::(response)? + ); + assert!(matches!( + commands.try_recv(), + Err(mpsc::error::TryRecvError::Empty) + )); + Ok(()) +} diff --git a/codex-rs/cloud-client/examples/attach.rs b/codex-rs/cloud-client/examples/attach.rs index 9cd4fcfab1..012b8fc48e 100644 --- a/codex-rs/cloud-client/examples/attach.rs +++ b/codex-rs/cloud-client/examples/attach.rs @@ -30,6 +30,8 @@ async fn main() -> anyhow::Result<()> { text_elements: vec![], }], turn_trigger: None, + model: None, + effort: None, mode: SubmissionMode::StartIfIdle, shared_access: false, }), diff --git a/codex-rs/cloud-client/src/client_tests.rs b/codex-rs/cloud-client/src/client_tests.rs index 820e861c8a..a7613d7834 100644 --- a/codex-rs/cloud-client/src/client_tests.rs +++ b/codex-rs/cloud-client/src/client_tests.rs @@ -174,6 +174,7 @@ async fn activation_preserves_native_settings_and_thread_metadata() { 0x2a, 1, b'/', // cwd (5) 0x52, 1, b'm', // model (10) 0x5a, 1, b'p', // model_provider (11) + 0x6a, 4, b'h', b'i', b'g', b'h', // reasoning_effort (13) 0x7a, 4, 0x12, 2, 8, 1, // sandbox (15): read-only with network access ]); prost::encoding::bytes::encode(/* thread */ 17, &thread, &mut response_bytes); @@ -206,6 +207,13 @@ async fn activation_preserves_native_settings_and_thread_metadata() { .await; let activation = server.client.activate("t").await.unwrap(); assert_eq!(activation.thread.protobuf().as_ref(), thread.as_slice()); + assert_eq!( + ( + activation.model.as_str(), + activation.reasoning_effort.as_deref() + ), + ("m", Some("high")), + ); let json = serde_json::to_value(&activation).unwrap(); assert_eq!(json["thread"]["source"], "unknown"); assert_eq!( @@ -671,3 +679,29 @@ mod transcript; #[path = "discovery_tests.rs"] mod discovery; + +#[test] +fn model_catalog_delivery_rejects_a_revoked_account() { + let controller = codex_http_client::NetworkPolicyController::default(); + let policy = controller.policy(); + assert!(controller.publish( + policy.revision(), + codex_http_client::DestinationPolicy::Unrestricted, + )); + let factory = HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault) + .with_network_policy(policy.clone().for_current_account()); + let client = Client::new( + &factory, + "https://example.com", + Credentials::new("synthetic-token", "synthetic-account").unwrap(), + ) + .unwrap(); + + assert!(client.model_catalog_is_current()); + policy.invalidate(); + assert!(controller.publish( + policy.revision(), + codex_http_client::DestinationPolicy::Unrestricted, + )); + assert!(!client.model_catalog_is_current()); +} diff --git a/codex-rs/cloud-client/src/lib.rs b/codex-rs/cloud-client/src/lib.rs index ce0d1a36e2..f6a814d801 100644 --- a/codex-rs/cloud-client/src/lib.rs +++ b/codex-rs/cloud-client/src/lib.rs @@ -82,9 +82,13 @@ pub use types::Event; pub use types::ResumeRequest; pub use types::RpcStatus; +use bytes::Bytes; +use bytes::BytesMut; use codex_http_client::ClientRouteClass; +use codex_http_client::HttpClient; use codex_http_client::HttpClientBuilder; use codex_http_client::HttpClientFactory; +use codex_http_client::NetworkPolicy; use codex_http_client::RouteAwareClientPool; use futures::StreamExt; use futures::stream::BoxStream; @@ -101,12 +105,18 @@ use url::Url; const REQUEST_TIMEOUT: Duration = Duration::from_secs(150); const MAX_MESSAGE_BYTES: usize = 64 * 1024 * 1024; +const MODEL_CATALOG_TIMEOUT: Duration = Duration::from_secs(10); +const MAX_MODEL_CATALOG_BYTES: usize = 1024 * 1024; type Transport = BoxCloneSyncService, http::Response, io::Error>; /// One fixed origin and account. Construct a new client when credentials change. /// Supply a trusted native gRPC origin, without a path (not the HTTP `/grpc` gateway). +#[derive(Clone)] pub struct Client { grpc: Grpc, + http: HttpClient, + model_catalog_url: Url, + policy: NetworkPolicy, credentials: Credentials, } @@ -181,8 +191,9 @@ impl Client { .as_str() .parse() .map_err(|_| Error::InvalidEndpoint)?; + let grpc_http = http.clone(); let transport = service_fn(move |request: http::Request| { - let http = http.clone(); + let http = grpc_http.clone(); async move { let (parts, body) = request.into_parts(); let response = http @@ -196,14 +207,71 @@ impl Client { Ok(response.into_http_response().map(Body::new)) } }); + let mut model_catalog_url = endpoint; + model_catalog_url.set_path("/v2/models"); Ok(Self { grpc: Grpc::with_origin(BoxCloneSyncService::new(transport), origin) .max_decoding_message_size(MAX_MESSAGE_BYTES) .max_encoding_message_size(MAX_MESSAGE_BYTES), + http, + model_catalog_url, + policy: factory.network_policy().clone(), credentials, }) } + /// Recheck the bound account before delivering an already completed catalog read. + pub fn model_catalog_is_current(&self) -> bool { + self.policy.acquire(&self.model_catalog_url).is_ok() + } + + /// Reads one account-scoped REST catalog page through the native client's transport. + /// The app-server adapter decodes the response with its existing catalog schema. + pub async fn list_models( + &self, + cursor: Option<&str>, + limit: Option, + include_hidden: Option, + ) -> Result { + let mut url = self.model_catalog_url.clone(); + { + let mut query = url.query_pairs_mut(); + if let Some(cursor) = cursor { + query.append_pair("cursor", cursor); + } + if let Some(limit) = limit { + query.append_pair("limit", &limit.to_string()); + } + if let Some(include_hidden) = include_hidden { + query.append_pair( + "includeHidden", + if include_hidden { "true" } else { "false" }, + ); + } + } + let mut response = self + .http + .get(url) + .headers(self.credentials.headers.clone()) + .timeout(MODEL_CATALOG_TIMEOUT) + .send() + .await + .and_then(codex_http_client::HttpResponse::error_for_status) + .map_err(|_| Error::ModelCatalogUnavailable)?; + let mut body = BytesMut::new(); + while let Some(chunk) = response + .chunk() + .await + .map_err(|_| Error::ModelCatalogUnavailable)? + { + if chunk.len() > MAX_MODEL_CATALOG_BYTES - body.len() { + return Err(Error::ModelCatalogTooLarge); + } + body.extend_from_slice(&chunk); + } + Ok(body.freeze()) + } + /// With `wait: false`, success means admission only. Failures may leave work admitted. pub async fn resume(&self, request: &ResumeRequest) -> Result<(), Error> { let mut grpc = self.grpc.clone(); diff --git a/codex-rs/cloud-client/src/turns.rs b/codex-rs/cloud-client/src/turns.rs index c16a570aae..eb9393e66c 100644 --- a/codex-rs/cloud-client/src/turns.rs +++ b/codex-rs/cloud-client/src/turns.rs @@ -18,7 +18,7 @@ pub enum SubmissionMode { Queue, StartIfIdle, } -/// Text and images using the existing thread configuration. Persist the message ID with +/// Text and images with optional conversation model settings. Persist the message ID with /// the input before calling; never reuse it for a different logical submission. #[derive(Clone)] pub struct AddUserMessageRequest { @@ -27,6 +27,9 @@ pub struct AddUserMessageRequest { pub input: Vec, /// Optional classification supplied by the caller for this turn. pub turn_trigger: Option, + /// Omit both to inherit the thread settings. A model alone uses its default effort. + pub model: Option, + pub effort: Option, pub mode: SubmissionMode, pub shared_access: bool, } @@ -125,6 +128,8 @@ impl Client { ) -> Result { if request.thread_id.trim().is_empty() || request.client_user_message_id.trim().is_empty() + || ((request.model.is_some() || request.effort.is_some()) + && matches!(request.mode, SubmissionMode::Queue)) || !request.input.iter().any(|input| match input { UserMessageInput::Text { text, .. } => !text.trim().is_empty(), UserMessageInput::Image { .. } => true, @@ -206,6 +211,8 @@ impl Client { }) .collect(), }), + model: request.model.clone(), + effort: request.effort.clone(), thread_id: request.thread_id.clone(), turn_trigger: request.turn_trigger.clone(), access: request.shared_access.then_some(1), diff --git a/codex-rs/cloud-client/src/turns_tests.rs b/codex-rs/cloud-client/src/turns_tests.rs index 6c17c26b07..34a6463e72 100644 --- a/codex-rs/cloud-client/src/turns_tests.rs +++ b/codex-rs/cloud-client/src/turns_tests.rs @@ -11,6 +11,8 @@ fn request() -> AddUserMessageRequest { text_elements: vec![], }], turn_trigger: None, + model: None, + effort: None, mode: SubmissionMode::StartIfIdle, shared_access: true, } @@ -26,6 +28,7 @@ fn admitted(outcome: u8) -> Vec { async fn submission_preserves_admission_variants_and_wire_identity() { for (mode, mode_number, outcome_number) in [ (SubmissionMode::StartIfIdle, 3, 1), + (SubmissionMode::Steer, 1, 1), (SubmissionMode::Steer, 1, 2), (SubmissionMode::Queue, 2, 3), (SubmissionMode::Queue, 2, 4), @@ -40,11 +43,14 @@ async fn submission_preserves_admission_variants_and_wire_identity() { } payload.extend([0xa0, 0x06, 7]); // Unknown metadata survives. let mut fixture = fixture(response(vec![frame(&payload)], tonic::Status::ok(""))).await; - let turn_trigger = (mode_number == 3).then(|| "safety_recourse_follow_up".to_owned()); + let has_selection = !matches!(mode, SubmissionMode::Queue); + let turn_trigger = (mode_number == 3).then(|| "user".to_owned()); let result = fixture .client .add_user_message(&AddUserMessageRequest { turn_trigger, + model: has_selection.then(|| "selected-model".into()), + effort: has_selection.then(|| "high".into()), mode, ..request() }) @@ -87,8 +93,11 @@ async fn submission_preserves_admission_variants_and_wire_identity() { format!("{result:?} {:?}", result.outcome), "Admission([redacted]) AdmissionOutcome([redacted])" ); + let effort = message(/*tag*/ 9, b"high"); + let model = message(/*tag*/ 12, b"selected-model"); let expected_request = [ message(/*tag*/ 4, b"m"), + if has_selection { effort } else { vec![] }, message( /*tag*/ 11, &message( @@ -96,9 +105,10 @@ async fn submission_preserves_admission_variants_and_wire_identity() { &message(/*tag*/ 1, &message(/*tag*/ 1, b"hello")), ), ), + if has_selection { model } else { vec![] }, message(/*tag*/ 23, b"t"), if mode_number == 3 { - message(/*tag*/ 25, b"safety_recourse_follow_up") + message(/*tag*/ 25, b"user") } else { Vec::new() }, @@ -279,6 +289,12 @@ async fn interrupt_addresses_original_turn_and_preserves_uncertain_errors() { async fn invalid_input_never_sends_and_diagnostics_redact() { let mut fixture = fixture(response(vec![frame(&[])], tonic::Status::ok(""))).await; for input in [ + AddUserMessageRequest { + mode: SubmissionMode::Queue, + effort: Some("high".into()), + model: Some("selected-model".into()), + ..request() + }, AddUserMessageRequest { input: vec![], ..request() diff --git a/codex-rs/cloud-client/src/turns_wire.rs b/codex-rs/cloud-client/src/turns_wire.rs index 377b18bc0b..34d1b2c1ac 100644 --- a/codex-rs/cloud-client/src/turns_wire.rs +++ b/codex-rs/cloud-client/src/turns_wire.rs @@ -5,8 +5,12 @@ use prost::Message; pub(super) struct AddRequest { #[prost(string, tag = "4")] pub client_user_message_id: String, + #[prost(string, optional, tag = "9")] + pub effort: Option, #[prost(message, optional, tag = "11")] pub input: Option, + #[prost(string, optional, tag = "12")] + pub model: Option, #[prost(string, tag = "23")] pub thread_id: String, #[prost(string, optional, tag = "25")] diff --git a/codex-rs/cloud-client/src/types.rs b/codex-rs/cloud-client/src/types.rs index a5b272842c..76c5e1328b 100644 --- a/codex-rs/cloud-client/src/types.rs +++ b/codex-rs/cloud-client/src/types.rs @@ -101,6 +101,10 @@ pub enum Error { Timeout, #[error("gRPC failed; request outcome may be unknown: {0:?}")] Rpc(RpcStatus), + #[error("Cloud model catalog is unavailable")] + ModelCatalogUnavailable, + #[error("Cloud model catalog exceeds the response limit")] + ModelCatalogTooLarge, #[error("invalid gRPC event envelope")] InvalidResponse, #[error("cloud response contains an unsupported {0}")] diff --git a/codex-rs/tui/src/app/cloud_switch_wait.rs b/codex-rs/tui/src/app/cloud_switch_wait.rs index fcd1ccf05f..9e332a7093 100644 --- a/codex-rs/tui/src/app/cloud_switch_wait.rs +++ b/codex-rs/tui/src/app/cloud_switch_wait.rs @@ -108,7 +108,7 @@ fn handle_cloud_switch_event( fn show_cloud_switch_hint(app: &mut App, progress: CloudSwitchProgress) { let (label, action) = match progress { - CloudSwitchProgress::Connecting(_) => ("Connecting to Cloud…", "submit when ready"), + CloudSwitchProgress::Connecting(_) => ("Connecting to Cloud…", ""), CloudSwitchProgress::Failed => ("Cloud conversation did not open", "draft kept"), }; app.chat_widget diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 7df9ffb6e2..76ce13ba55 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -2604,6 +2604,12 @@ impl App { } AppEvent::ModelsLoaded { request_id, result } | AppEvent::ApiKeyCompatModelsLoaded { request_id, result } => { + let result = match app_server.request_handle() { + AppServerRequestHandle::Cloud(handle) if !handle.model_catalog_is_current() => { + Err("Cloud model catalog account changed.".into()) + } + _ => result, + }; self.chat_widget.on_models_loaded(request_id, result); let catalog = self.chat_widget.model_catalog(); if !Arc::ptr_eq(&self.model_catalog, &catalog) { @@ -2687,8 +2693,13 @@ impl App { } } AppEvent::OpenPlanReasoningScopePrompt { model, effort } => { - self.chat_widget - .open_plan_reasoning_scope_prompt(model, effort); + if app_server.thread_params_mode() == crate::ThreadParamsMode::Cloud { + self.select_session_model(app_server, model, effort, /*service_tier*/ None) + .await; + } else { + self.chat_widget + .open_plan_reasoning_scope_prompt(model, effort); + } } AppEvent::OpenAllModelsPopup => { self.chat_widget.open_all_models_popup(); diff --git a/codex-rs/tui/src/app/model_defaults.rs b/codex-rs/tui/src/app/model_defaults.rs index 240700e2e9..39bfaff64b 100644 --- a/codex-rs/tui/src/app/model_defaults.rs +++ b/codex-rs/tui/src/app/model_defaults.rs @@ -5,6 +5,7 @@ use super::App; use crate::app_server_session::AppServerSession; +use crate::app_server_session::ThreadParamsMode; use crate::app_server_session::ThreadSettingsOwner; use crate::service_tier_resolution; use codex_app_server_client::AppServerRequestHandle; @@ -13,6 +14,7 @@ use codex_app_server_protocol::WriteStatus; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::config_types::ServiceTier; +use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::ReasoningEffort; use color_eyre::eyre::Result; @@ -91,6 +93,49 @@ impl App { effort: Option, service_tier: Option, ) { + if app_server.thread_params_mode() == ThreadParamsMode::Cloud { + let Some(thread_id) = self + .active_thread_id + .filter(|id| Some(*id) == self.chat_widget.thread_id()) + else { + self.chat_widget.add_error_message( + "Open the Cloud conversation before choosing a model.".into(), + ); + return; + }; + if service_tier.is_some() + || model == crate::model_catalog::LUNA_RESERVE_MODEL + || !self.model_catalog.models.iter().any(|preset| { + preset.model == model + && effort.as_ref().is_none_or(|effort| { + *effort == preset.default_reasoning_effort + || preset + .supported_reasoning_efforts + .iter() + .any(|preset| &preset.effort == effort) + }) + && preset.model_specialty.as_deref() != Some(MODEL_SPECIALTY_CYBER) + }) + { + self.chat_widget.add_error_message( + "Choose a supported Cloud model and effort at the current speed.".into(), + ); + return; + } + let label = Self::reasoning_label_for(&model, effort.as_ref()) + .map_or_else(String::new, |label| format!(" {label}")); + let message = + format!("Selected {model}{label} for the next new turn in this conversation"); + if app_server.stage_cloud_model_selection(thread_id, model, effort) { + self.chat_widget.add_info_message(message, /*hint*/ None); + } else { + self.chat_widget.add_error_message( + "The Cloud account changed. Reopen the conversation before choosing a model." + .into(), + ); + } + return; + } // Changing effort or speed for the effective model must preserve permissions. let model_changed = self.chat_widget.current_model() != model; let params = if model_changed { diff --git a/codex-rs/tui/src/app/tests/model_defaults_tests.rs b/codex-rs/tui/src/app/tests/model_defaults_tests.rs index c1637e6d43..33a99c33da 100644 --- a/codex-rs/tui/src/app/tests/model_defaults_tests.rs +++ b/codex-rs/tui/src/app/tests/model_defaults_tests.rs @@ -1,6 +1,8 @@ use super::*; use crate::collaboration_modes; use codex_app_server_client::AppServerClient; +use codex_app_server_client::CloudOperation; +use codex_app_server_client::CloudUserMessageReceipt; use codex_config::LoaderOverrides; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use crossterm::event::KeyCode; @@ -155,24 +157,28 @@ async fn standard_speed_selection_falls_back_for_older_app_servers() -> Result<( } #[tokio::test] -async fn cloud_model_selection_requires_server_acceptance() -> Result<()> { - for event in [ - AppEvent::SelectSessionModel { - model: "gpt-5.6-terra".into(), - effort: Some(ReasoningEffortConfig::High), - service_tier: None, - }, - AppEvent::ApplyModelSelection { - model: "gpt-5.6-terra".into(), - effort: Some(ReasoningEffortConfig::High), - }, - AppEvent::ApplyAdvancedReasoning { - model: "gpt-5.6-terra".into(), - effort: ReasoningEffortConfig::Ultra, - }, +async fn cloud_model_selection_waits_for_native_admission() -> Result<()> { + let turn = test_turn("cloud-turn", TurnStatus::InProgress, Vec::new()); + for receipt in [ + Ok(CloudUserMessageReceipt::Started(turn.clone())), + Ok(CloudUserMessageReceipt::Steered(turn)), + Err(codex_app_server_protocol::JSONRPCErrorError { + code: -32602, + message: "Cloud reply attachment failed local preflight".into(), + data: None, + }), + Err(codex_app_server_protocol::JSONRPCErrorError { + code: -32000, + message: "Native mutation outcome unknown".into(), + data: None, + }), ] { - let (mut app, mut events, _ops) = make_test_app_with_channels().await; + let admitted = receipt.is_ok(); + let unconfirmed = receipt.as_ref().is_err_and(|error| error.code == -32000); + let (mut app, mut events, mut ops) = make_test_app_with_channels().await; let thread_id = ThreadId::new(); + app.chat_widget + .handle_thread_session(test_thread_session(thread_id, app.config.cwd.to_path_buf())); app.active_thread_id = Some(thread_id); let original_model = app.chat_widget.current_model().to_string(); let original_effort = app.chat_widget.current_reasoning_effort(); @@ -186,36 +192,13 @@ async fn cloud_model_selection_requires_server_acceptance() -> Result<()> { let mut tui = crate::tui::test_support::make_test_tui()?; while events.try_recv().is_ok() {} - let (result, operation) = - tokio::join!(app.handle_event(&mut tui, &mut server, event), async { - let command = commands.recv().await.expect("cloud settings update"); - let codex_app_server_client::CloudOperation::Request(request) = command.operation - else { - panic!("expected cloud request") - }; - let ClientRequest::ThreadSettingsUpdate { params, .. } = *request else { - panic!("expected thread/settings/update request") - }; - command - .response - .send(Ok(Err(JSONRPCErrorError { - code: -32601, - data: None, - message: "method not found".to_string(), - }))) - .expect("model selection should still await its response"); - params - }); - - result?; - assert_eq!(operation.thread_id, thread_id.to_string()); - assert_eq!( - ( - app.chat_widget.current_model(), - app.chat_widget.current_reasoning_effort(), - ), - (original_model.as_str(), original_effort) - ); + let event = AppEvent::ApplyModelSelection { + model: "gpt-5.6-terra".into(), + effort: Some(ReasoningEffortConfig::High), + }; + Box::pin(app.handle_event(&mut tui, &mut server, event)).await?; + assert!(commands.try_recv().is_err()); + assert!(app.chat_widget.cloud_picker_selection.is_none()); let messages = std::iter::from_fn(|| events.try_recv().ok()) .filter_map(|event| match event { AppEvent::InsertHistoryCell(cell) => { @@ -227,9 +210,67 @@ async fn cloud_model_selection_requires_server_acceptance() -> Result<()> { insta::allow_duplicates! { insta::assert_snapshot!( messages.join("\n"), - @"■ This cloud conversation does not support changing its model from the TUI." + @"• Selected gpt-5.6-terra high for the next new turn in this conversation" ); } + app.chat_widget.apply_external_edit("continue".into()); + app.chat_widget.handle_key_event(KeyCode::Esc.into()); + app.chat_widget.handle_key_event(KeyCode::Enter.into()); + let op = next_user_turn_op(&mut ops); + let admit = async { + let command = commands.recv().await.expect("native admission request"); + let CloudOperation::SubmitUserMessage(params) = command.operation else { + panic!("expected StartOrSteer"); + }; + assert_eq!( + (params.model.as_deref(), params.effort), + (Some("gpt-5.6-terra"), Some(ReasoningEffortConfig::High)) + ); + let response = receipt.map(|receipt| serde_json::to_value(receipt).unwrap()); + command.response.send(Ok(response)).unwrap(); + }; + let submit = Box::pin(super::super::user_turn_submission::try_submit_user_turn( + &mut app, + &mut server, + thread_id, + &op, + )); + let (result, ()) = tokio::time::timeout(std::time::Duration::from_secs(5), async { + tokio::join!(submit, admit) + }) + .await?; + assert!(result?); + assert_eq!( + ( + app.chat_widget.current_model(), + app.chat_widget.current_reasoning_effort(), + ), + (original_model.as_str(), original_effort), + ); + let input = app.chat_widget.capture_thread_input_state(); + let held = input.is_some_and(|input| input.has_unconfirmed_messages()); + assert_eq!(held, unconfirmed); + let rejected = !admitted && !unconfirmed; + let restored = if rejected { "continue" } else { "" }; + assert_eq!(app.chat_widget.composer_text_with_pending(), restored); + let accepted = app.chat_widget.cloud_picker_selection.as_ref(); + assert_eq!( + accepted.map(|selection| (selection.model.as_str(), selection.effort.clone())), + admitted.then_some(("gpt-5.6-terra", Some(ReasoningEffortConfig::High))), + ); + if accepted.is_some() { + app.chat_widget.open_model_popup(); + let popup = render_bottom_popup(&app.chat_widget, /*width*/ 80); + let label = popup.lines().find(|line| line.contains("Accepted for")); + insta::allow_duplicates! { + insta::assert_snapshot!(label.unwrap().trim(), @"Accepted for future turns: gpt-5.6-terra · High"); + } + app.chat_widget.handle_thread_session(test_thread_session( + thread_id, + app.config.cwd.to_path_buf(), + )); + assert!(app.chat_widget.cloud_picker_selection.is_none()); + } } Ok(()) } diff --git a/codex-rs/tui/src/app/tests/new_session_tests.rs b/codex-rs/tui/src/app/tests/new_session_tests.rs index cbda37f346..3d229fa491 100644 --- a/codex-rs/tui/src/app/tests/new_session_tests.rs +++ b/codex-rs/tui/src/app/tests/new_session_tests.rs @@ -1279,7 +1279,7 @@ async fn cloud_switch_wait_displays_pending_submission_hint() -> Result<()> { .find(|line| line.contains("Connecting to Cloud")) .expect("Cloud connection hint") .trim(), - @"Connecting to Cloud… submit when ready" + @"Connecting to Cloud…" ); Ok(()) } diff --git a/codex-rs/tui/src/app/user_turn_submission.rs b/codex-rs/tui/src/app/user_turn_submission.rs index 5b13691c0b..07e5f78b22 100644 --- a/codex-rs/tui/src/app/user_turn_submission.rs +++ b/codex-rs/tui/src/app/user_turn_submission.rs @@ -19,7 +19,6 @@ pub(super) async fn try_submit_user_turn( turn_trigger, cloud_submission, client_user_message_id, - model, .. } = op else { @@ -47,7 +46,7 @@ pub(super) async fn try_submit_user_turn( }; let original_active = app.active_turn_id_for_thread(thread_id).await; let mut active = original_active.clone(); - let preflight = || { + let preflight = |model: &str| { let enabled = app.chat_widget.daybreak_enabled && !app.chat_widget.side_conversation_active() && !app.side_threads.contains_key(&thread_id); @@ -94,7 +93,10 @@ pub(super) async fn try_submit_user_turn( } match result { - Ok(admission) => { + Ok((admission, selection)) => { + if selection.is_some() { + app.chat_widget.cloud_picker_selection = selection; + } if !matches!(&admission, UserMessageAdmission::Queued(_)) { crate::startup_recovery::acknowledged(client_user_message_id); } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 5c593bd92b..11a777a112 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -399,6 +399,7 @@ pub(crate) struct AppServerSession { pub(crate) model_provider_override: Option, history_support: ThreadHistorySupport, pub(crate) thread_settings_update_supported: bool, + cloud_model_selections: HashMap, default_model: Option, available_models: Vec, prefetched_model_list: Option>>>, @@ -618,6 +619,7 @@ impl AppServerSession { model_provider_override: None, history_support: ThreadHistorySupport::Paginated, thread_settings_update_supported: true, + cloud_model_selections: HashMap::new(), default_model: None, available_models: Vec::new(), prefetched_model_list: None, @@ -883,6 +885,10 @@ impl AppServerSession { let handle = self.request_handle(); let request_id = self.next_request_id(); async move { + if mode == ThreadParamsMode::Cloud { + // Cloud activation supplies the model. Fetch the catalog on picker demand. + return Ok(Vec::new()); + } models::list(mode, handle, request_id).await.map_err(|err| { bootstrap_request_error("model/list failed during TUI bootstrap", err) }) @@ -1814,7 +1820,7 @@ impl AppServerSession { } #[allow(clippy::too_many_arguments)] - pub(crate) async fn turn_start( + async fn turn_start( &mut self, thread_id: ThreadId, turn_trigger: &str, @@ -1832,8 +1838,8 @@ impl AppServerSession { collaboration_mode: Option, output_schema: Option, cyber_access_program: Option, + cloud_selection: Option<&user_message_submission::CloudModelSelection>, ) -> Result { - let request_id = self.next_request_id(); let mut params = TurnStartParams { thread_id: thread_id.to_string(), turn_trigger: Some(turn_trigger.to_string()), @@ -1857,7 +1863,11 @@ impl AppServerSession { params.effort = effort; params.summary = summary; params.collaboration_mode = collaboration_mode; + } else if let Some(selection) = cloud_selection { + params.model = Some(selection.model.clone()); + params.effort.clone_from(&selection.effort); } + let request_id = self.next_request_id(); self.request_handle .request_typed(ClientRequest::TurnStart { request_id, params }) .await @@ -3734,6 +3744,7 @@ mod tests { }), Some(output_schema.clone()), Some(CyberAccessProgram::DaybreakBlue.into()), + /*cloud_selection*/ None, ), async { let command = commands.recv().await.expect("cloud turn start"); diff --git a/codex-rs/tui/src/app_server_session/models.rs b/codex-rs/tui/src/app_server_session/models.rs index c504b78989..c35323a954 100644 --- a/codex-rs/tui/src/app_server_session/models.rs +++ b/codex-rs/tui/src/app_server_session/models.rs @@ -44,11 +44,7 @@ pub(super) async fn list( request_handle: AppServerRequestHandle, request_id: RequestId, ) -> Result, TypedRequestError> { - if mode == ThreadParamsMode::Cloud { - // The internal preview uses the model returned by Cloud when the thread opens. - return Ok(Vec::new()); - } - let response: ModelListResponse = request_handle + let response: ModelListResponse = match request_handle .request_typed(ClientRequest::ModelList { request_id, params: ModelListParams { @@ -58,10 +54,24 @@ pub(super) async fn list( model_provider: None, }, }) - .await?; + .await + { + Ok(response) => response, + // Older Cloud servers do not expose model discovery. Preserve their startup behavior. + Err(TypedRequestError::Server { source, .. }) + if mode == ThreadParamsMode::Cloud && source.code == -32601 => + { + return Ok(Vec::new()); + } + Err(error) => return Err(error), + }; Ok(response .data .into_iter() .map(model_preset_from_api_model) .collect()) } + +#[cfg(test)] +#[path = "models_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app_server_session/models_tests.rs b/codex-rs/tui/src/app_server_session/models_tests.rs new file mode 100644 index 0000000000..b790a4eb24 --- /dev/null +++ b/codex-rs/tui/src/app_server_session/models_tests.rs @@ -0,0 +1,84 @@ +#![cfg(test)] +//! Keep Cloud discovery deferred and preserve typed model-list behavior on other targets. + +use super::*; +use crate::backend_id::BackendId; +use assert_matches::assert_matches; +use codex_app_server_client::AppServerClient; +use codex_app_server_client::CloudClient; +use codex_app_server_client::CloudOperation; +use codex_app_server_protocol::JSONRPCErrorError; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[tokio::test] +async fn model_catalog_prefetch_and_refresh_use_server_discovery() -> color_eyre::Result<()> { + let model = json!({ + "id": "cloud-model", "model": "cloud-model", "displayName": "Cloud Model", + "description": "A model supplied by the connected server", "hidden": false, + "supportedReasoningEfforts": [{"reasoningEffort": "medium", "description": "Medium"}], + "defaultReasoningEffort": "medium", "isDefault": true, + }); + let preset = model_preset_from_api_model(serde_json::from_value(model.clone())?); + for (mode, error_code) in [ + (ThreadParamsMode::Cloud, None), + (ThreadParamsMode::Remote, None), + (ThreadParamsMode::Embedded, None), + (ThreadParamsMode::Cloud, Some(-32601)), + (ThreadParamsMode::Remote, Some(-32601)), + (ThreadParamsMode::Cloud, Some(-32603)), + ] { + let cloud = mode == ThreadParamsMode::Cloud; + let (client, mut commands, _events) = CloudClient::channel(/*capacity*/ 1); + let response = match error_code { + None => Ok(json!({"data": [model.clone()], "nextCursor": null})), + Some(code) => Err(JSONRPCErrorError { + code, + message: "Discovery unavailable".into(), + data: None, + }), + }; + let worker = tokio::spawn(async move { + for _ in 0..if cloud { 1 } else { 2 } { + let command = commands.recv().await.expect("model list request"); + let CloudOperation::Request(request) = command.operation else { + panic!("expected model/list"); + }; + let params = + assert_matches!(*request, ClientRequest::ModelList { params, .. } => params); + let expected = ModelListParams { + include_hidden: Some(true), + ..Default::default() + }; + assert_eq!(params, expected); + command.response.send(Ok(response.clone())).unwrap(); + } + }); + let client = AppServerClient::Cloud(client); + let mut session = AppServerSession::new(BackendId::new(), client, mode); + session.prefetch_model_list(); + let initial = session.prefetched_model_list.take().unwrap().await; + match error_code { + _ if cloud => assert!(initial?.is_empty()), + None => assert_eq!(initial?, vec![preset.clone()]), + Some(_) => assert!(initial.is_err()), + } + + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let request_id = Uuid::new_v4(); + session.fetch_models(request_id, AppEventSender::new(tx)); + let event = tokio::time::timeout(std::time::Duration::from_secs(10), rx.recv()).await?; + let (loaded, result) = assert_matches!( + event, + Some(AppEvent::ModelsLoaded { request_id, result }) => (request_id, result) + ); + assert_eq!(loaded, request_id); + match error_code { + None => assert_eq!(result, Ok(vec![preset.clone()])), + Some(-32601) if cloud => assert_eq!(result, Ok(Vec::new())), + Some(_) => assert!(result.is_err()), + } + worker.await?; + } + Ok(()) +} diff --git a/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs b/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs index e93065510d..8909026d58 100644 --- a/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs +++ b/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs @@ -153,6 +153,7 @@ stream_max_retries = 0 /*collaboration_mode*/ None, /*output_schema*/ None, /*cyber_access_program*/ None, + /*cloud_selection*/ None, ) .await?; tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 30), async { diff --git a/codex-rs/tui/src/app_server_session/user_message_submission.rs b/codex-rs/tui/src/app_server_session/user_message_submission.rs index d213a7104d..8bcfcd5775 100644 --- a/codex-rs/tui/src/app_server_session/user_message_submission.rs +++ b/codex-rs/tui/src/app_server_session/user_message_submission.rs @@ -10,6 +10,7 @@ use codex_app_server_protocol::CodexErrorInfo as AppServerCodexErrorInfo; use codex_app_server_protocol::TurnError as AppServerTurnError; use codex_app_server_protocol::TurnStartParams; use codex_protocol::ThreadId; +use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::turn_input::CyberAccessProgram; use codex_utils_absolute_path::AbsolutePathBuf; use color_eyre::eyre::Report; @@ -76,16 +77,43 @@ impl UserMessageSubmissionFailure { } } +#[derive(Clone)] +pub(crate) struct CloudModelSelection { + pub(crate) model: String, + pub(crate) effort: Option, +} + impl AppServerSession { + fn cloud_model_catalog_is_current(&self) -> bool { + matches!(&self.request_handle, + AppServerRequestHandle::Cloud(handle) if handle.model_catalog_is_current() + ) + } + + pub(crate) fn stage_cloud_model_selection( + &mut self, + thread_id: ThreadId, + model: String, + effort: Option, + ) -> bool { + if !self.cloud_model_catalog_is_current() { + return false; + } + self.cloud_model_selections + .insert(thread_id, CloudModelSelection { model, effort }); + true + } + pub(crate) async fn submit_user_message<'a>( &mut self, thread_id: ThreadId, op: &AppCommand, intent: CloudSubmissionIntent, active_turn: &mut Option, - preflight: impl FnOnce() -> Result, String>, + preflight: impl FnOnce(&str) -> Result, String>, start_settings: impl FnOnce() -> TurnStartSettings<'a>, - ) -> Result { + ) -> Result<(UserMessageAdmission, Option), UserMessageSubmissionFailure> + { let AppCommand::UserTurn { turn_trigger, client_user_message_id, @@ -120,11 +148,13 @@ impl AppServerSession { ) .await { - Ok(response) => return Ok(UserMessageAdmission::Steered(response.turn_id)), + Ok(response) => { + return Ok((UserMessageAdmission::Steered(response.turn_id), None)); + } Err(error) => error, }; if let Some(error) = active_turn_not_steerable_turn_error(&error) { - return Ok(UserMessageAdmission::Queued(error.message)); + return Ok((UserMessageAdmission::Queued(error.message), None)); } match active_turn_steer_race(&error) { Some(ActiveTurnSteerRace::Missing) => { @@ -148,29 +178,58 @@ impl AppServerSession { } } - let program = preflight().map_err(UserMessageSubmissionFailure::Preflight)?; + let selection = + if cloud && *turn_trigger == "user" && intent != CloudSubmissionIntent::UseKnownTurn { + self.cloud_model_selections.get(&thread_id).cloned() + } else { + None + }; + if selection.is_some() && !self.cloud_model_catalog_is_current() { + self.cloud_model_selections.remove(&thread_id); + return Err(UserMessageSubmissionFailure::Preflight( + "The Cloud account changed. Reopen the conversation before sending.".into(), + )); + } + let selected_model = selection + .as_ref() + .map_or(model.as_str(), |selection| selection.model.as_str()); + let program = preflight(selected_model).map_err(UserMessageSubmissionFailure::Preflight)?; + let take_selection = |session: &mut Self| { + selection.as_ref()?; + session + .cloud_model_selections + .remove(&thread_id) + .filter(|_| session.cloud_model_catalog_is_current()) + }; if cloud && intent == CloudSubmissionIntent::StartOrSteer && let AppServerRequestHandle::Cloud(handle) = &self.request_handle { + let (model, effort) = selection.as_ref().map_or((None, None), |selection| { + (Some(selection.model.clone()), selection.effort.clone()) + }); let result = handle .submit_user_message(TurnStartParams { thread_id: thread_id.to_string(), + turn_trigger: model.as_ref().map(|_| turn_trigger.to_string()), client_user_message_id: Some(client_user_message_id.clone()), input: items.clone(), output_schema: final_output_json_schema.clone(), + model, + effort, ..Default::default() }) .await; - return match result { + let admission = match result { Ok(CloudUserMessageReceipt::Started(turn)) => { Ok(UserMessageAdmission::Started(turn.id)) } + // A successful steer saves these settings for subsequent starts. Ok(CloudUserMessageReceipt::Steered(turn)) => { Ok(UserMessageAdmission::Steered(turn.id)) } Ok(CloudUserMessageReceipt::Duplicate { .. }) => { - Ok(UserMessageAdmission::Duplicate) + return Ok((UserMessageAdmission::Duplicate, None)); } // The adapter returns invalid params only for failures before admission. Err(TypedRequestError::Server { source, .. }) @@ -184,7 +243,8 @@ impl AppServerSession { error: error.into(), reconnect: false, }), - }; + }?; + return Ok((admission, take_selection(self))); } let settings = start_settings(); self.turn_start( @@ -204,9 +264,13 @@ impl AppServerSession { collaboration_mode.clone(), final_output_json_schema.clone(), program.map(Into::into), + selection.as_ref(), ) .await - .map(|response| UserMessageAdmission::StartResponse(response.turn.id)) + .map(|response| { + let admission = UserMessageAdmission::StartResponse(response.turn.id); + (admission, take_selection(self)) + }) .map_err( |error| match UserMessageSubmissionFailure::conventional(error, intent) { UserMessageSubmissionFailure::Rejected(error) => { diff --git a/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs b/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs index 2b8005a7ed..5a22921782 100644 --- a/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs +++ b/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs @@ -110,6 +110,7 @@ stream_max_retries = 0 /*collaboration_mode*/ None, /*output_schema*/ None, /*cyber_access_program*/ None, + /*cloud_selection*/ None, ) .await?; tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 30), async { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 063e56f048..e748352efd 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -550,6 +550,8 @@ pub(crate) struct ChatWidget { has_codex_backend_auth: bool, model_catalog: Arc, model_popup_request_id: Option, + pub(crate) cloud_picker_selection: + Option, /// An app event has been queued to open the permissions picker, but no view exists yet. pub(crate) permission_popup_pending: bool, permission_popup_request_id: Option, diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index 3eb899d334..1ae43235f0 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -134,6 +134,7 @@ impl ChatWidget { has_codex_backend_auth, model_catalog, model_popup_request_id: None, + cloud_picker_selection: None, permission_popup_pending: false, permission_popup_request_id: None, permission_discovery: None, diff --git a/codex-rs/tui/src/chatwidget/model_popup_state.rs b/codex-rs/tui/src/chatwidget/model_popup_state.rs index 6367e0feb3..f2690504a9 100644 --- a/codex-rs/tui/src/chatwidget/model_popup_state.rs +++ b/codex-rs/tui/src/chatwidget/model_popup_state.rs @@ -2,6 +2,7 @@ //! Preserve highlighted models by wire slug, independent of their display names. use super::model_popups::ALL_MODELS_SELECTION_VIEW_ID; +use super::model_popups::MODEL_LOADING_VIEW_ID; use super::model_popups::MODEL_OPTIONS_VIEW_ID; use super::model_popups::MODEL_SELECTION_VIEW_ID; use super::*; @@ -29,13 +30,19 @@ impl ChatWidget { pub(crate) fn dismiss_stale_model_picker(&mut self) { self.model_popup_request_id = None; + self.cloud_picker_selection = None; + let had_popup = !self.no_modal_or_popup_active(); for id in [ + MODEL_LOADING_VIEW_ID, MODEL_OPTIONS_VIEW_ID, ALL_MODELS_SELECTION_VIEW_ID, MODEL_SELECTION_VIEW_ID, ] { while self.bottom_pane.dismiss_view_by_id(id) {} } + if had_popup && self.no_modal_or_popup_active() { + self.app_event_tx.send(AppEvent::SettingsSelectionClosed); + } } pub(super) fn refresh_account_scoped_models(&mut self) { @@ -77,12 +84,21 @@ impl ChatWidget { return false; } self.model_popup_request_id = None; + let loading = self.bottom_pane.active_view_id() == Some(MODEL_LOADING_VIEW_ID); + self.bottom_pane.dismiss_view_by_id(MODEL_LOADING_VIEW_ID); + if loading { + match &result { + Ok(presets) => self.open_model_popup_with_presets(presets.clone()), + Err(_) => self.add_error_message("Could not load models. Try /model again.".into()), + } + if self.no_modal_or_popup_active() { + self.on_modal_or_popup_closed(); + } + } let Ok(presets) = result else { return false; }; - if presets.is_empty() - || self.model_catalog.try_list_models().ok().as_ref() == Some(&presets) - { + if presets.is_empty() || self.model_catalog.models == presets { return false; } Arc::make_mut(&mut self.model_catalog).models = presets; diff --git a/codex-rs/tui/src/chatwidget/model_popups.rs b/codex-rs/tui/src/chatwidget/model_popups.rs index afaca38867..977f185a22 100644 --- a/codex-rs/tui/src/chatwidget/model_popups.rs +++ b/codex-rs/tui/src/chatwidget/model_popups.rs @@ -8,6 +8,7 @@ use crate::model_catalog::LUNA_RESERVE_MODEL; const ULTRA_REASONING_CONCURRENCY_WARNING_THRESHOLD: usize = 8; pub(super) const MODEL_OPTIONS_VIEW_ID: &str = "model-options"; +pub(super) const MODEL_LOADING_VIEW_ID: &str = "model-loading"; pub(super) const MODEL_SELECTION_VIEW_ID: &str = "model-selection"; pub(super) const ALL_MODELS_SELECTION_VIEW_ID: &str = "all-models-selection"; @@ -40,7 +41,15 @@ impl ChatWidget { }; let request_id = uuid::Uuid::new_v4(); self.model_popup_request_id = Some(request_id); - self.open_model_popup_with_presets(presets); + if presets.is_empty() { + self.bottom_pane.show_selection_view(SelectionViewParams { + view_id: Some(MODEL_LOADING_VIEW_ID), + title: Some("Loading models…".into()), + ..SelectionViewParams::picker() + }); + } else { + self.open_model_popup_with_presets(presets); + } // Show cached choices immediately and update any still-present picker when the reply arrives. self.app_event_tx.send(AppEvent::FetchModels { request_id }); } @@ -52,6 +61,17 @@ impl ChatWidget { if !subtitle.is_empty() { header.push(Line::from(subtitle.dim())); } + if let Some(selection) = &self.cloud_picker_selection { + let effort = selection + .effort + .as_ref() + .map_or_else(|| "server effort".to_string(), Self::reasoning_effort_label); + header.push( + format!("Accepted for future turns: {} · {effort}", selection.model) + .dim() + .into(), + ); + } if let Some(warning) = self.model_menu_warning_line() { header.push(warning); } diff --git a/codex-rs/tui/src/chatwidget/session_flow.rs b/codex-rs/tui/src/chatwidget/session_flow.rs index 57376049a2..398fa0013a 100644 --- a/codex-rs/tui/src/chatwidget/session_flow.rs +++ b/codex-rs/tui/src/chatwidget/session_flow.rs @@ -90,6 +90,9 @@ impl ChatWidget { self.set_skills(/*skills*/ None); self.session_network_proxy = session.network_proxy.clone(); let previous_thread_id = self.thread_id; + if previous_thread_id != Some(session.thread_id) || self.cloud_picker_selection.is_some() { + self.dismiss_stale_model_picker(); + } let connector_scope_changed = previous_thread_id != Some(session.thread_id) || self.config.cwd.as_path() != session.cwd.as_path(); self.thread_id = Some(session.thread_id); diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index 18c805904a..bc61ae5d15 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -254,7 +254,7 @@ impl ChatWidget { ) { // Account-update notifications are the identity boundary. The visible account fields can // be identical across two accounts, so always invalidate account-scoped requests and data. - self.model_popup_request_id = None; + self.dismiss_stale_model_picker(); let api_key_changed = self.status_account_display == Some(StatusAccountDisplay::ApiKey) || status_account_display == Some(StatusAccountDisplay::ApiKey); self.invalidate_permission_discovery(); diff --git a/codex-rs/tui/src/chatwidget/tests/model_picker_tests.rs b/codex-rs/tui/src/chatwidget/tests/model_picker_tests.rs index 613c1b2e87..3e2fec17c5 100644 --- a/codex-rs/tui/src/chatwidget/tests/model_picker_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/model_picker_tests.rs @@ -18,3 +18,23 @@ async fn model_picker_compact_hint_and_actions_follow_configured_bindings() { chat.handle_key_event(KeyEvent::from(KeyCode::F(2))); assert!(chat.no_modal_or_popup_active()); } + +#[tokio::test] +async fn model_picker_opens_after_discovery_populates_an_empty_catalog() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.5")).await; + chat.thread_id = Some(ThreadId::new()); + let presets = std::mem::take(&mut Arc::make_mut(&mut chat.model_catalog).models); + chat.open_model_popup(); + let loading = render_bottom_popup(&chat, /*width*/ 80); + let loading_title = loading.lines().find(|line| line.contains("Loading models")); + insta::assert_snapshot!(loading_title.unwrap().trim(), @"Loading models…"); + let obsolete = chat.model_popup_request_id.unwrap(); + chat.handle_key_event(KeyCode::Esc.into()); + assert!(chat.no_modal_or_popup_active()); + chat.open_model_popup(); + assert!(!chat.on_models_loaded(obsolete, Ok(presets.clone()))); + let request_id = chat.model_popup_request_id.unwrap(); + assert!(chat.on_models_loaded(request_id, Ok(presets))); + let popup = render_bottom_popup(&chat, /*width*/ 80); + assert_chatwidget_snapshot!("model_selection_popup", popup); +}