mirror of
https://github.com/openai/codex.git
synced 2026-11-06 00:30:29 +00:00
Consolidate thread startup around StartThreadOptions (#34814)
## What changed - Add `StartThreadOptions::new` to provide the standard configuration for a new thread. - Make `ThreadManager::start_thread` the single thread-start entry point and migrate callers from the previous convenience methods. - Derive default environment selections when `environments` is `None`, while preserving explicit selections, including an empty list. GitOrigin-RevId: 8977dc11aed54c5e1215a81eaed2b2cf5fc6087a
This commit is contained in:
1 parent
06782eded7
commit
08ae0fc0ce
25 files changed
+237
-399
No files matched your search
@@ -8,7 +8,6 @@ use codex_exec_server::REMOTE_ENVIRONMENT_ID;
|
||||
use codex_features::Feature;
|
||||
use codex_home::CodexHomeUserInstructionsProvider;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
@@ -484,19 +483,9 @@ async fn loads_user_instructions_without_a_primary_environment() -> Result<()> {
|
||||
|
||||
let no_environment_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: test.config.clone(),
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
session_source: None,
|
||||
thread_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments: Vec::new(),
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
.start_thread(StartThreadOptions {
|
||||
environments: Some(Vec::new()),
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(provider.load_count(), 2);
|
||||
@@ -691,17 +680,8 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho
|
||||
let remote_source = test.config.cwd.join(GLOBAL_AGENTS_FILENAME);
|
||||
let thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: test.config.clone(),
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
session_source: None,
|
||||
thread_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments: vec![
|
||||
.start_thread(StartThreadOptions {
|
||||
environments: Some(vec![
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
cwd: PathUri::from_abs_path(&test.config.cwd),
|
||||
@@ -712,9 +692,8 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho
|
||||
cwd: PathUri::from_host_native_path(local_root.path())?,
|
||||
workspace_roots: vec![PathUri::from_host_native_path(local_root.path())?],
|
||||
},
|
||||
],
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
]),
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(provider.load_count(), 2);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem;
|
||||
use codex_protocol::dynamic_tools::DynamicToolFunctionSpec;
|
||||
use codex_protocol::dynamic_tools::DynamicToolNamespaceSpec;
|
||||
@@ -94,7 +95,10 @@ async fn dynamic_tool_audio_exceeding_the_output_budget_is_omitted() -> Result<(
|
||||
});
|
||||
let new_thread = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(base_test.config.clone(), vec![dynamic_tool])
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![dynamic_tool],
|
||||
..StartThreadOptions::new(base_test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
let mut test = base_test;
|
||||
test.codex = new_thread.thread;
|
||||
|
||||
@@ -4,6 +4,7 @@ use codex_core::ModelClient;
|
||||
use codex_core::NewThread;
|
||||
use codex_core::Prompt;
|
||||
use codex_core::ResponseEvent;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::resolve_installation_id;
|
||||
use codex_core::thread_store_from_config;
|
||||
@@ -1741,7 +1742,7 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() {
|
||||
/*external_time_provider*/ None,
|
||||
);
|
||||
let NewThread { thread: codex, .. } = thread_manager
|
||||
.start_thread(config.clone())
|
||||
.start_thread(StartThreadOptions::new(config.clone()))
|
||||
.await
|
||||
.expect("create new conversation");
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::CurrentTimeReminderConfig;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
@@ -3628,9 +3629,8 @@ async fn code_mode_can_call_hidden_dynamic_tools() -> Result<()> {
|
||||
let base_test = builder.build(&server).await?;
|
||||
let new_thread = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(
|
||||
base_test.config.clone(),
|
||||
vec![DynamicToolSpec::Namespace(DynamicToolNamespaceSpec {
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![DynamicToolSpec::Namespace(DynamicToolNamespaceSpec {
|
||||
name: "codex_app".to_string(),
|
||||
description: "Codex app tools.".to_string(),
|
||||
tools: vec![DynamicToolNamespaceTool::Function(
|
||||
@@ -3649,7 +3649,8 @@ async fn code_mode_can_call_hidden_dynamic_tools() -> Result<()> {
|
||||
},
|
||||
)],
|
||||
})],
|
||||
)
|
||||
..StartThreadOptions::new(base_test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
let mut test = base_test;
|
||||
test.codex = new_thread.thread;
|
||||
@@ -3797,9 +3798,8 @@ async fn code_mode_excludes_configured_nested_tool_namespaces() -> Result<()> {
|
||||
let base_test = builder.build(&server).await?;
|
||||
let new_thread = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(
|
||||
base_test.config.clone(),
|
||||
vec![DynamicToolSpec::Namespace(DynamicToolNamespaceSpec {
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![DynamicToolSpec::Namespace(DynamicToolNamespaceSpec {
|
||||
name: "excluded".to_string(),
|
||||
description: "Excluded tools.".to_string(),
|
||||
tools: vec![DynamicToolNamespaceTool::Function(
|
||||
@@ -3815,7 +3815,8 @@ async fn code_mode_excludes_configured_nested_tool_namespaces() -> Result<()> {
|
||||
},
|
||||
)],
|
||||
})],
|
||||
)
|
||||
..StartThreadOptions::new(base_test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
let mut test = base_test;
|
||||
test.codex = new_thread.thread;
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::fs;
|
||||
use anyhow::Result;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::compact::SUMMARY_PREFIX;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
@@ -1287,7 +1288,10 @@ async fn remote_compact_filters_deferred_dynamic_tools() -> Result<()> {
|
||||
})];
|
||||
let new_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(test.config.clone(), dynamic_tools)
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools,
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
test.codex = new_thread.thread;
|
||||
test.session_configured = new_thread.session_configured;
|
||||
@@ -1403,7 +1407,10 @@ async fn remote_compact_does_not_charge_inline_audio_payload_as_text() -> Result
|
||||
});
|
||||
let new_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(test.config.clone(), vec![dynamic_tool])
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![dynamic_tool],
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
test.codex = new_thread.thread;
|
||||
test.session_configured = new_thread.session_configured;
|
||||
@@ -2045,7 +2052,10 @@ async fn remote_compact_trims_tool_search_output_to_empty_tools_array() -> Resul
|
||||
let mut test = builder.build(&server).await?;
|
||||
let new_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(test.config.clone(), vec![dynamic_tool])
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![dynamic_tool],
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
test.codex = new_thread.thread;
|
||||
test.session_configured = new_thread.session_configured;
|
||||
|
||||
@@ -11,7 +11,6 @@ use codex_protocol::config_types::ApprovalsReviewer;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -257,24 +256,11 @@ async fn ephemeral_system_thread_prewarm_skips_and_turn_observes_fresh_state() -
|
||||
|
||||
let mut config = test.config.clone();
|
||||
config.ephemeral = true;
|
||||
let environments = test
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd, &config.workspace_roots);
|
||||
let system_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config,
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
session_source: None,
|
||||
.start_thread(StartThreadOptions {
|
||||
thread_source: Some(ThreadSource::Feature("system".to_string())),
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments,
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
..StartThreadOptions::new(config)
|
||||
})
|
||||
.await?;
|
||||
let prewarm = tokio::time::timeout(
|
||||
@@ -386,47 +372,21 @@ async fn concurrent_turns_keep_distinct_worktree_and_repository_metadata() -> Re
|
||||
|
||||
let mut worktree_config = test.config.clone();
|
||||
worktree_config.cwd = worktree.abs();
|
||||
let worktree_environments = test
|
||||
.thread_manager
|
||||
.default_environment_selections(&worktree_config.cwd, &worktree_config.workspace_roots);
|
||||
let worktree_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: worktree_config,
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
session_source: None,
|
||||
.start_thread(StartThreadOptions {
|
||||
thread_source: Some(ThreadSource::User),
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments: worktree_environments,
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
..StartThreadOptions::new(worktree_config)
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut other_config = test.config.clone();
|
||||
other_config.cwd = other_repo.path().to_path_buf().abs();
|
||||
let other_environments = test
|
||||
.thread_manager
|
||||
.default_environment_selections(&other_config.cwd, &other_config.workspace_roots);
|
||||
let other_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: other_config,
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
session_source: None,
|
||||
.start_thread(StartThreadOptions {
|
||||
thread_source: Some(ThreadSource::User),
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments: other_environments,
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
..StartThreadOptions::new(other_config)
|
||||
})
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -19,7 +19,6 @@ use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
@@ -1415,19 +1414,10 @@ async fn session_end_skips_subagents() -> Result<()> {
|
||||
] {
|
||||
let subagent = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: test.config.clone(),
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
.start_thread(StartThreadOptions {
|
||||
session_source: Some(SessionSource::SubAgent(source)),
|
||||
thread_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments: Vec::new(),
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
environments: Some(Vec::new()),
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use codex_core::NewThread;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -183,7 +184,7 @@ async fn regular_mcp_definition_cache_preserves_live_session_state() -> anyhow::
|
||||
..
|
||||
} = fixture
|
||||
.thread_manager
|
||||
.start_thread(fixture.config.clone())
|
||||
.start_thread(StartThreadOptions::new(fixture.config.clone()))
|
||||
.await?;
|
||||
let second_pid = wait_for_new_pid(fs.as_ref(), &pid_file, Some(&first_pid)).await?;
|
||||
let second_process = process_label(&second_pid);
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
use anyhow::Result;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem;
|
||||
use codex_protocol::dynamic_tools::DynamicToolFunctionSpec;
|
||||
@@ -939,7 +940,10 @@ async fn tool_search_returns_deferred_dynamic_tool_and_routes_follow_up_call() -
|
||||
let base_test = builder.build(&server).await?;
|
||||
let new_thread = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(base_test.config.clone(), vec![dynamic_tool])
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![dynamic_tool],
|
||||
..StartThreadOptions::new(base_test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
let mut test = base_test;
|
||||
test.codex = new_thread.thread;
|
||||
@@ -1571,7 +1575,10 @@ async fn tool_search_matches_dynamic_tools_by_name_description_namespace_and_sch
|
||||
let base_test = builder.build(&server).await?;
|
||||
let new_thread = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(base_test.config.clone(), vec![dynamic_tool])
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![dynamic_tool],
|
||||
..StartThreadOptions::new(base_test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
let mut test = base_test;
|
||||
test.codex = new_thread.thread;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::config::Config;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_features::Feature;
|
||||
@@ -154,7 +155,10 @@ async fn resume_restores_dynamic_tools_from_rollout_with_sqlite_enabled() -> Res
|
||||
let base_test = builder.build(&server).await?;
|
||||
let started = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(base_test.config.clone(), vec![dynamic_tool])
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![dynamic_tool],
|
||||
..StartThreadOptions::new(base_test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
let rollout_path = started
|
||||
.session_configured
|
||||
@@ -251,7 +255,7 @@ async fn resume_restores_legacy_dynamic_tools_from_rollout_with_sqlite_enabled()
|
||||
let base_test = builder.build(&server).await?;
|
||||
let started = base_test
|
||||
.thread_manager
|
||||
.start_thread_with_tools(base_test.config.clone(), Vec::new())
|
||||
.start_thread(StartThreadOptions::new(base_test.config.clone()))
|
||||
.await?;
|
||||
let rollout_path = started
|
||||
.session_configured
|
||||
|
||||
@@ -10,7 +10,6 @@ use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
@@ -797,19 +796,10 @@ async fn subagent_stop_replaces_stop_and_skips_internal_subagents() -> Result<()
|
||||
// because the SubagentStop hook above intentionally matches all agent types.
|
||||
let internal_thread = test
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: test.config.clone(),
|
||||
allow_provider_model_fallback: false,
|
||||
initial_history: InitialHistory::New,
|
||||
history_mode: None,
|
||||
.start_thread(StartThreadOptions {
|
||||
session_source: Some(SessionSource::SubAgent(SubAgentSource::Review)),
|
||||
thread_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
metrics_service_name: None,
|
||||
parent_trace: None,
|
||||
environments: Vec::new(),
|
||||
thread_extension_init: Default::default(),
|
||||
supports_openai_form_elicitation: false,
|
||||
environments: Some(Vec::new()),
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
|
||||
|
||||
Reference in new issue
Block a user