mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
Treat managed new-thread settings as defaults
This commit is contained in:
@@ -3,6 +3,7 @@ use crate::error_code::method_not_found;
|
||||
use codex_app_server_protocol::SelectedCapabilityRoot;
|
||||
use codex_extension_api::ExtensionDataInit;
|
||||
use codex_protocol::config_types::MultiAgentMode;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
|
||||
@@ -1044,6 +1045,17 @@ impl ThreadRequestProcessor {
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let thread_start_started_at = std::time::Instant::now();
|
||||
let requested_cwd = typesafe_overrides.cwd.clone();
|
||||
let has_explicit_model = typesafe_overrides.model.is_some()
|
||||
|| config_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("model"));
|
||||
let has_explicit_reasoning_effort = config_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("model_reasoning_effort"));
|
||||
let has_explicit_service_tier = typesafe_overrides.service_tier.is_some()
|
||||
|| config_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("service_tier"));
|
||||
let mut config = config_manager
|
||||
.load_with_overrides(config_overrides.clone(), typesafe_overrides.clone())
|
||||
.await
|
||||
@@ -1114,6 +1126,31 @@ impl ThreadRequestProcessor {
|
||||
.map_err(|err| config_load_error(&err))?;
|
||||
}
|
||||
|
||||
if let Some(defaults) = config
|
||||
.config_layer_stack
|
||||
.requirements_toml()
|
||||
.models
|
||||
.as_ref()
|
||||
.and_then(|models| models.new_thread.as_ref())
|
||||
{
|
||||
if !has_explicit_model && let Some(model) = defaults.model.as_ref() {
|
||||
config.model = Some(model.clone());
|
||||
}
|
||||
if !has_explicit_reasoning_effort
|
||||
&& let Some(reasoning_effort) = defaults.model_reasoning_effort.as_ref()
|
||||
{
|
||||
config.model_reasoning_effort = Some(reasoning_effort.clone());
|
||||
}
|
||||
if !has_explicit_service_tier && let Some(service_tier) = defaults.service_tier.as_ref()
|
||||
{
|
||||
config.service_tier = Some(
|
||||
ServiceTier::from_request_value(service_tier)
|
||||
.map(|tier| tier.request_value().to_string())
|
||||
.unwrap_or_else(|| service_tier.clone()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let environments = environments.unwrap_or_else(|| {
|
||||
listener_task_context
|
||||
.thread_manager
|
||||
|
||||
@@ -37,6 +37,7 @@ use codex_protocol::openai_models::ReasoningEffort;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::TempDir;
|
||||
@@ -201,17 +202,18 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_uses_managed_new_thread_model_defaults() -> Result<()> {
|
||||
async fn thread_start_treats_managed_new_thread_model_settings_as_defaults() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?;
|
||||
let managed_model = all_model_presets()
|
||||
.iter()
|
||||
.find(|preset| {
|
||||
preset
|
||||
.service_tiers
|
||||
.iter()
|
||||
.any(|tier| tier.id == "priority")
|
||||
preset.id != "gpt-5.2"
|
||||
&& preset
|
||||
.service_tiers
|
||||
.iter()
|
||||
.any(|tier| tier.id == "priority")
|
||||
})
|
||||
.context("expected a model with the fast service tier")?;
|
||||
std::fs::write(
|
||||
@@ -230,22 +232,59 @@ service_tier = "fast"
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
let default_request_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams::default())
|
||||
.await?;
|
||||
let default_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(default_request_id)),
|
||||
)
|
||||
.await??;
|
||||
let default_response: ThreadStartResponse = to_response(default_response)?;
|
||||
|
||||
assert_eq!(
|
||||
(
|
||||
default_response.model.as_str(),
|
||||
default_response.reasoning_effort,
|
||||
default_response.service_tier.as_deref(),
|
||||
),
|
||||
(
|
||||
managed_model.id.as_str(),
|
||||
Some(ReasoningEffort::Medium),
|
||||
Some("priority"),
|
||||
)
|
||||
);
|
||||
|
||||
let explicit_request_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("gpt-requested".to_string()),
|
||||
model: Some("gpt-5.2".to_string()),
|
||||
service_tier: Some(Some(SERVICE_TIER_DEFAULT_REQUEST_VALUE.to_string())),
|
||||
config: Some(HashMap::from([(
|
||||
"model_reasoning_effort".to_string(),
|
||||
json!("low"),
|
||||
)])),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
let explicit_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(explicit_request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: ThreadStartResponse = to_response(response)?;
|
||||
let explicit_response: ThreadStartResponse = to_response(explicit_response)?;
|
||||
|
||||
assert_eq!(response.model, managed_model.id);
|
||||
assert_eq!(response.reasoning_effort, Some(ReasoningEffort::Medium));
|
||||
assert_eq!(response.service_tier.as_deref(), Some("priority"));
|
||||
assert_eq!(
|
||||
(
|
||||
explicit_response.model.as_str(),
|
||||
explicit_response.reasoning_effort,
|
||||
explicit_response.service_tier.as_deref(),
|
||||
),
|
||||
(
|
||||
"gpt-5.2",
|
||||
Some(ReasoningEffort::Low),
|
||||
Some(SERVICE_TIER_DEFAULT_REQUEST_VALUE),
|
||||
)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -41,7 +41,6 @@ use codex_models_manager::manager::SharedModelsManager;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::CollaborationModeMask;
|
||||
use codex_protocol::config_types::MultiAgentMode;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::Result as CodexResult;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
@@ -218,45 +217,6 @@ fn effective_originator_value(
|
||||
.unwrap_or(default_originator)
|
||||
}
|
||||
|
||||
fn apply_managed_new_thread_defaults(
|
||||
config: &mut Config,
|
||||
initial_history: &InitialHistory,
|
||||
session_source: &SessionSource,
|
||||
) {
|
||||
if session_source.is_non_root_agent()
|
||||
|| !matches!(
|
||||
initial_history,
|
||||
InitialHistory::New | InitialHistory::Cleared
|
||||
)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(defaults) = config
|
||||
.config_layer_stack
|
||||
.requirements_toml()
|
||||
.models
|
||||
.as_ref()
|
||||
.and_then(|models| models.new_thread.as_ref())
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
if let Some(model) = defaults.model.as_ref() {
|
||||
config.model = Some(model.clone());
|
||||
}
|
||||
if let Some(model_reasoning_effort) = defaults.model_reasoning_effort.as_ref() {
|
||||
config.model_reasoning_effort = Some(model_reasoning_effort.clone());
|
||||
}
|
||||
if let Some(service_tier) = defaults.service_tier.as_ref() {
|
||||
config.service_tier = Some(
|
||||
ServiceTier::from_request_value(service_tier)
|
||||
.map(|tier| tier.request_value().to_string())
|
||||
.unwrap_or_else(|| service_tier.clone()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) struct ResumeThreadWithHistoryOptions {
|
||||
pub(crate) config: Config,
|
||||
pub(crate) initial_history: InitialHistory,
|
||||
@@ -1538,7 +1498,7 @@ impl ThreadManagerState {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn spawn_thread_with_source(
|
||||
&self,
|
||||
mut config: Config,
|
||||
config: Config,
|
||||
initial_history: InitialHistory,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
agent_control: AgentControl,
|
||||
@@ -1579,7 +1539,6 @@ impl ThreadManagerState {
|
||||
threads.remove(&resumed.conversation_id);
|
||||
}
|
||||
}
|
||||
apply_managed_new_thread_defaults(&mut config, &initial_history, &session_source);
|
||||
let user_instructions = self
|
||||
.user_instructions_for_spawn(&session_source, parent_thread_id, forked_from_thread_id)
|
||||
.await;
|
||||
|
||||
@@ -16,7 +16,6 @@ use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ReasoningItemReasoningSummary;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AgentMessageEvent;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::InternalSessionSource;
|
||||
@@ -79,60 +78,6 @@ impl codex_agent_graph_store::AgentGraphStore for FakeAgentGraphStore {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_new_thread_defaults_only_apply_to_root_new_threads() {
|
||||
let mut base_config = test_config().await;
|
||||
base_config.model = Some("local-model".to_string());
|
||||
base_config.model_reasoning_effort = Some(ReasoningEffort::Low);
|
||||
base_config.service_tier = Some("flex".to_string());
|
||||
base_config.config_layer_stack = codex_config::ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
codex_config::ConfigRequirements::default(),
|
||||
codex_config::ConfigRequirementsToml {
|
||||
models: Some(codex_config::ModelsRequirementsToml {
|
||||
new_thread: Some(codex_config::NewThreadModelDefaultsToml {
|
||||
model: Some("managed-model".to_string()),
|
||||
model_reasoning_effort: Some(ReasoningEffort::Medium),
|
||||
service_tier: Some("fast".to_string()),
|
||||
}),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("build managed requirements stack");
|
||||
|
||||
let cases = [
|
||||
(
|
||||
InitialHistory::New,
|
||||
SessionSource::VSCode,
|
||||
("managed-model", ReasoningEffort::Medium, "priority"),
|
||||
),
|
||||
(
|
||||
InitialHistory::Cleared,
|
||||
SessionSource::VSCode,
|
||||
("managed-model", ReasoningEffort::Medium, "priority"),
|
||||
),
|
||||
(
|
||||
InitialHistory::Forked(Vec::new()),
|
||||
SessionSource::VSCode,
|
||||
("local-model", ReasoningEffort::Low, "flex"),
|
||||
),
|
||||
(
|
||||
InitialHistory::New,
|
||||
SessionSource::SubAgent(SubAgentSource::Review),
|
||||
("local-model", ReasoningEffort::Low, "flex"),
|
||||
),
|
||||
];
|
||||
|
||||
for (history, source, (expected_model, expected_effort, expected_service_tier)) in cases {
|
||||
let mut config = base_config.clone();
|
||||
apply_managed_new_thread_defaults(&mut config, &history, &source);
|
||||
assert_eq!(config.model.as_deref(), Some(expected_model));
|
||||
assert_eq!(config.model_reasoning_effort, Some(expected_effort));
|
||||
assert_eq!(config.service_tier.as_deref(), Some(expected_service_tier));
|
||||
}
|
||||
}
|
||||
|
||||
fn user_msg(text: &str) -> ResponseItem {
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
|
||||
Reference in New Issue
Block a user