mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
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
This commit is contained in:
committed by
copyberry
parent
a92eae0891
commit
82e6bacb9c
@@ -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());
|
||||
}
|
||||
|
||||
@@ -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::Request<Body>, http::Response<Body>, 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<Transport>,
|
||||
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<Body>| {
|
||||
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<u32>,
|
||||
include_hidden: Option<bool>,
|
||||
) -> Result<Bytes, Error> {
|
||||
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();
|
||||
|
||||
@@ -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<UserMessageInput>,
|
||||
/// Optional classification supplied by the caller for this turn.
|
||||
pub turn_trigger: Option<String>,
|
||||
/// Omit both to inherit the thread settings. A model alone uses its default effort.
|
||||
pub model: Option<String>,
|
||||
pub effort: Option<String>,
|
||||
pub mode: SubmissionMode,
|
||||
pub shared_access: bool,
|
||||
}
|
||||
@@ -125,6 +128,8 @@ impl Client {
|
||||
) -> Result<Admission, MutationError> {
|
||||
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),
|
||||
|
||||
@@ -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<u8> {
|
||||
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()
|
||||
|
||||
@@ -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<String>,
|
||||
#[prost(message, optional, tag = "11")]
|
||||
pub input: Option<Input>,
|
||||
#[prost(string, optional, tag = "12")]
|
||||
pub model: Option<String>,
|
||||
#[prost(string, tag = "23")]
|
||||
pub thread_id: String,
|
||||
#[prost(string, optional, tag = "25")]
|
||||
|
||||
@@ -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}")]
|
||||
|
||||
Reference in New Issue
Block a user