mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Use thread-scoped model catalogs for model lookups (#61421)
## Why Model lookups through the startup catalog can use the wrong provider's capabilities or instructions after a default provider switch or when a thread has its own catalog. ## What changed - Resolve model metadata for sampling, prediction forks, and memory writes through the owning thread's catalog. - Make the thread's models manager available to extensions before startup callbacks and use it for guardian sampler configuration. - Import external sessions using the destination provider and originator's catalog, loading its offline cache only when no refreshed catalog is available. ## Testing Add regression coverage for sampling after a default provider switch, parent-specific prediction fork capabilities, thread-scoped model lookups, and imported sessions retaining destination provider instructions and originator. GitOrigin-RevId: c86081b9e55c5e2b42a7f082966485d7fb5139d4
This commit is contained in:
@@ -432,12 +432,23 @@ impl ExternalAgentSessionImporter {
|
||||
format!("failed to load imported session config: {err}"),
|
||||
)
|
||||
})?;
|
||||
let models_manager = self.thread_manager.get_models_manager();
|
||||
let originator = codex_login::default_client::originator().value;
|
||||
let models_manager = self.thread_manager.models_manager_for_config(&config);
|
||||
let models_manager = models_manager
|
||||
.for_originator(&originator)
|
||||
.unwrap_or(models_manager);
|
||||
// A pinned model skips discovery. Load its destination cache without fetching or
|
||||
// replacing a provider response that is already in memory.
|
||||
if models_manager.refreshed_model_catalog().is_none() {
|
||||
models_manager
|
||||
.list_models(RefreshStrategy::Offline, config.http_client_factory())
|
||||
.await;
|
||||
}
|
||||
let model = models_manager
|
||||
.get_default_model(
|
||||
&config.model,
|
||||
/*allow_provider_model_fallback*/ false,
|
||||
RefreshStrategy::Offline,
|
||||
RefreshStrategy::InMemory,
|
||||
config.http_client_factory(),
|
||||
)
|
||||
.await;
|
||||
@@ -465,7 +476,7 @@ impl ExternalAgentSessionImporter {
|
||||
parent_thread_id: None,
|
||||
source: source.clone(),
|
||||
thread_source: None,
|
||||
originator: codex_login::default_client::originator().value,
|
||||
originator,
|
||||
base_instructions: BaseInstructions {
|
||||
text: config
|
||||
.base_instructions
|
||||
|
||||
@@ -5435,10 +5435,8 @@ impl ThreadRequestProcessor {
|
||||
.items()
|
||||
.any(|item| matches!(item, ResponseItem::AdditionalTools { .. }))
|
||||
&& let Some(model) = options.config.model.as_deref()
|
||||
&& self
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info(model, &options.config.to_models_manager_config())
|
||||
&& parent
|
||||
.model_info(model, &options.config.to_models_manager_config())
|
||||
.await
|
||||
.use_responses_lite
|
||||
{
|
||||
|
||||
@@ -182,9 +182,8 @@ impl SampleTask {
|
||||
})
|
||||
.await
|
||||
.map_err(|error| internal_error(error.to_string()))?;
|
||||
let model = manager
|
||||
.get_models_manager()
|
||||
.get_model_info(
|
||||
let model = parent
|
||||
.model_info(
|
||||
options.config.model.as_deref().unwrap_or(&snapshot.model),
|
||||
&options.config.to_models_manager_config(),
|
||||
)
|
||||
|
||||
@@ -1918,7 +1918,39 @@ async fn external_agent_config_import_sends_completion_notification_after_pendin
|
||||
async fn external_agent_config_import_creates_session_rollouts() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("follow-up answer").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
MockResponsesConfig::new(&server.uri()).write(codex_home.path())?;
|
||||
let url = server.uri();
|
||||
let instructions = "Instructions from the destination provider.";
|
||||
let mut model = codex_models_manager::model_info::model_info_from_slug("mock-model");
|
||||
model.visibility = codex_protocol::openai_models::ModelVisibility::List;
|
||||
model.model_messages = Some(codex_protocol::openai_models::ModelMessages {
|
||||
instructions_template: Some(instructions.into()),
|
||||
..Default::default()
|
||||
});
|
||||
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||
.and(wiremock::matchers::path("/v1/models"))
|
||||
.and(wiremock::matchers::header(
|
||||
"originator",
|
||||
"codex_work_desktop",
|
||||
))
|
||||
.respond_with(
|
||||
wiremock::ResponseTemplate::new(200)
|
||||
.set_body_json(serde_json::json!({"models": [model]})),
|
||||
)
|
||||
// The responses fixture also registers a default empty catalog.
|
||||
.with_priority(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
MockResponsesConfig::new(&url)
|
||||
.enable_feature(codex_features::Feature::ApiKeyModelDiscovery)
|
||||
.with_extra_config(&format!(
|
||||
r#"[model_providers.destination]
|
||||
name = "Destination"
|
||||
base_url = "{url}/v1"
|
||||
model_catalog_url = "{url}/v1/models"
|
||||
http_headers = {{ originator = "unscoped-test" }}
|
||||
env_key = "TEST_CATALOG_KEY""#
|
||||
))
|
||||
.write(codex_home.path())?;
|
||||
let project_root = codex_home.path().join("repo");
|
||||
let source_created_at_text = "2024-01-02T03:04:05Z";
|
||||
let source_updated_at_text = "2024-03-01T04:05:06Z";
|
||||
@@ -1979,10 +2011,40 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_auto_env()
|
||||
.with_env_overrides(&[("HOME", Some(home_dir.as_str()))])
|
||||
.build_initialized_with_timeout(DEFAULT_TIMEOUT)
|
||||
.with_env_overrides(&[
|
||||
("HOME", Some(home_dir.as_str())),
|
||||
("TEST_CATALOG_KEY", Some("test-key")),
|
||||
])
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
mcp.initialize_with_client_info(codex_app_server_protocol::ClientInfo {
|
||||
name: "codex_work_desktop".into(),
|
||||
title: None,
|
||||
version: "0.1.0".into(),
|
||||
})
|
||||
.await?;
|
||||
let id = mcp
|
||||
.send_raw_request(
|
||||
"modelProvider/default/set",
|
||||
Some(serde_json::json!({"providerId": "destination"})),
|
||||
)
|
||||
.await?;
|
||||
let write: codex_app_server_protocol::ConfigWriteResponse = mcp.read_response(id).await?;
|
||||
assert_eq!(write.status, codex_app_server_protocol::WriteStatus::Ok);
|
||||
// Warm the partition the imported chat will use on resume.
|
||||
let id = mcp
|
||||
.send_thread_start_request(codex_app_server_protocol::ThreadStartParams {
|
||||
ephemeral: Some(true),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let started: codex_app_server_protocol::ThreadStartResponse = mcp.read_response(id).await?;
|
||||
assert_eq!(
|
||||
started.thread.originator.as_deref(),
|
||||
Some("codex_work_desktop")
|
||||
);
|
||||
|
||||
let request_id = mcp
|
||||
.send_raw_request(
|
||||
"externalAgentConfig/detect",
|
||||
@@ -2077,7 +2139,7 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> {
|
||||
limit: None,
|
||||
sort_key: None,
|
||||
sort_direction: None,
|
||||
model_providers: None,
|
||||
model_providers: Some(vec!["destination".into()]),
|
||||
source_kinds: None,
|
||||
archived: None,
|
||||
section_id: None,
|
||||
@@ -2097,6 +2159,18 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> {
|
||||
.expect("expected imported thread")
|
||||
.clone();
|
||||
assert_eq!(imported_thread_id, thread.id.to_string());
|
||||
assert_eq!(thread.model_provider, "destination");
|
||||
let meta =
|
||||
codex_rollout::read_session_meta_line(thread.path.as_deref().expect("imported rollout"))
|
||||
.await?;
|
||||
assert_eq!(meta.meta.originator, "codex_work_desktop");
|
||||
assert_eq!(
|
||||
meta.meta
|
||||
.base_instructions
|
||||
.expect("imported instructions")
|
||||
.text,
|
||||
instructions
|
||||
);
|
||||
assert_eq!(thread.preview, control_request);
|
||||
assert_eq!(thread.name.as_deref(), Some("Fix auth flow"));
|
||||
assert_eq!(thread.created_at, source_created_at);
|
||||
|
||||
@@ -3095,10 +3095,7 @@ async fn fork_option_disables_mcp_servers(
|
||||
)?;
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_model("gpt-5.4")
|
||||
.with_root_config(&format!(
|
||||
"model_catalog_json = {}\nfeatures.incremental_tools = {incremental_tools}",
|
||||
serde_json::to_string(&catalog)?
|
||||
))
|
||||
.with_root_config(&format!("features.incremental_tools = {incremental_tools}"))
|
||||
.with_extra_config(&format!(
|
||||
"[mcp_servers.fixture]\nurl = \"{}/mcp\"\nrequired = true\n",
|
||||
mcp_server.uri()
|
||||
@@ -3111,6 +3108,8 @@ async fn fork_option_disables_mcp_servers(
|
||||
let parent = app
|
||||
.start_thread(ThreadStartParams {
|
||||
history_mode: Some(history_mode),
|
||||
// Only the parent catalog advertises the capability used by the prediction fork.
|
||||
config: Some([("model_catalog_json".into(), json!(catalog))].into()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -1348,6 +1348,95 @@ async fn sample_rejects_invalid_sources_and_prompts() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sample_uses_the_parent_catalog_after_a_default_provider_switch() -> Result<()> {
|
||||
let alpha = wiremock::MockServer::start().await;
|
||||
let beta = wiremock::MockServer::start().await;
|
||||
let mut model = codex_models_manager::bundled_models_response()?
|
||||
.models
|
||||
.into_iter()
|
||||
.find(|model| model.slug == "gpt-5.5")
|
||||
.expect("bundled reasoning model");
|
||||
for (provider, supports_updates) in [(&alpha, false), (&beta, true)] {
|
||||
model.supports_reasoning_effort_updates = supports_updates;
|
||||
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||
.and(wiremock::matchers::path("/v1/models"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({"models": [&model]})))
|
||||
.mount(provider)
|
||||
.await;
|
||||
}
|
||||
let replies = ["parent", "sample"].map(|id| {
|
||||
responses::sse(vec![
|
||||
responses::ev_assistant_message(id, "Ready."),
|
||||
responses::ev_completed(id),
|
||||
])
|
||||
});
|
||||
let alpha_requests = responses::mount_sse_sequence(&alpha, replies.to_vec()).await;
|
||||
let beta_requests = responses::mount_sse_sequence(&beta, replies.to_vec()).await;
|
||||
let home = TempDir::new()?;
|
||||
let alpha_url = alpha.uri();
|
||||
let beta_url = beta.uri();
|
||||
MockResponsesConfig::new(&alpha_url)
|
||||
.with_model_provider("alpha")
|
||||
.with_provider_name("OpenAI")
|
||||
.with_model(&model.slug)
|
||||
.with_root_config("model_reasoning_effort = 'medium'")
|
||||
.enable_feature(Feature::ApiKeyModelDiscovery)
|
||||
.enable_feature(Feature::ReasoningEffortOverride)
|
||||
.with_provider_config(&format!(
|
||||
"env_key = 'TEST_CATALOG_KEY'\nmodel_catalog_url = '{alpha_url}/v1/models'"
|
||||
))
|
||||
.with_extra_config(&format!(
|
||||
r#"[model_providers.beta]
|
||||
name = "OpenAI"
|
||||
base_url = "{beta_url}/v1"
|
||||
model_catalog_url = "{beta_url}/v1/models"
|
||||
env_key = "TEST_CATALOG_KEY""#
|
||||
))
|
||||
.write(home.path())?;
|
||||
let mut client = TestAppServer::builder()
|
||||
.with_codex_home(home.path())
|
||||
.with_env_overrides(&[("TEST_CATALOG_KEY", Some("test-key"))])
|
||||
.build_initialized_with_timeout(WAIT)
|
||||
.await?;
|
||||
let alpha_parent = ready_parent(&mut client, ThreadStartParams::default()).await?;
|
||||
let id = client
|
||||
.send_raw_request(
|
||||
"modelProvider/default/set",
|
||||
Some(json!({"providerId": "beta"})),
|
||||
)
|
||||
.await?;
|
||||
let write: codex_app_server_protocol::ConfigWriteResponse = client.read_response(id).await?;
|
||||
assert_eq!(write.status, codex_app_server_protocol::WriteStatus::Ok);
|
||||
let beta_parent = ready_parent(&mut client, ThreadStartParams::default()).await?;
|
||||
for ((thread_id, expected_turn_id), requests, supports_updates) in [
|
||||
(alpha_parent, alpha_requests, false),
|
||||
(beta_parent, beta_requests, true),
|
||||
] {
|
||||
sample(
|
||||
&mut client,
|
||||
ThreadSampleParams {
|
||||
thread_id,
|
||||
expected_turn_id,
|
||||
prompt: "Predict".into(),
|
||||
effort: Some(ReasoningEffort::High),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
let requests = requests.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let updates = requests[1].inputs_of_type("configuration_update");
|
||||
assert_eq!(
|
||||
updates
|
||||
.iter()
|
||||
.any(|item| item["reasoning"]["effort"] == "high"),
|
||||
supports_updates
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false; "unsupported model")]
|
||||
#[test_case(true; "unsupported provider")]
|
||||
#[tokio::test]
|
||||
|
||||
@@ -25,6 +25,7 @@ use codex_extension_api::ConversationHistorySnapshot;
|
||||
use codex_extension_api::ThreadIdleCause;
|
||||
use codex_features::Feature;
|
||||
use codex_history::RolloutItem;
|
||||
use codex_models_manager::ModelsManagerConfig;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_otel::current_span_w3c_trace_context;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -41,6 +42,7 @@ use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ProfileWorkspaceRoot;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::CatalogModel;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -1205,6 +1207,15 @@ impl CodexThread {
|
||||
self.session.selected_model_info().await
|
||||
}
|
||||
|
||||
/// Looks up a named model in the catalog selected for this thread's provider and credentials.
|
||||
pub async fn model_info(&self, model: &str, config: &ModelsManagerConfig) -> CatalogModel {
|
||||
self.session
|
||||
.services
|
||||
.models_manager
|
||||
.get_model_info(model, config)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn config_snapshot(&self) -> ThreadConfigSnapshot {
|
||||
self.session.thread_config_snapshot().await
|
||||
}
|
||||
|
||||
@@ -42,6 +42,8 @@ mod compact_remote_v2;
|
||||
mod compact_retained_tool_outputs;
|
||||
mod compact_token_budget;
|
||||
mod thread_startup_metadata;
|
||||
/// Catalog handle installed in each thread's extension data before startup callbacks.
|
||||
pub use codex_models_manager::manager::SharedModelsManager;
|
||||
pub use codex_network_proxy::EnvironmentNetworkPolicy;
|
||||
pub use codex_network_proxy::NetworkDomainPermission;
|
||||
pub use codex_network_proxy::NetworkDomainPermissionEntry;
|
||||
|
||||
@@ -1054,6 +1054,7 @@ impl Session {
|
||||
thread_extension_init.insert(codex_extension_api::ThreadOriginator(
|
||||
session_configuration.originator.clone(),
|
||||
));
|
||||
thread_extension_init.insert(Arc::clone(&models_manager));
|
||||
// Publish the already resolved model before extensions make startup decisions.
|
||||
// Turn construction refreshes this attachment when the selected model changes.
|
||||
thread_extension_init.insert(Arc::unwrap_or_clone(model_info.model));
|
||||
|
||||
@@ -1107,6 +1107,8 @@ impl ThreadManager {
|
||||
Arc::clone(&self.state.git_root_discovery)
|
||||
}
|
||||
|
||||
/// Returns the startup catalog. Use [`CodexThread::model_info`] for a live chat or
|
||||
/// [`Self::models_manager_for_config`] for a new destination.
|
||||
pub fn get_models_manager(&self) -> SharedModelsManager {
|
||||
self.state.model_catalog.manager.clone()
|
||||
}
|
||||
|
||||
@@ -436,8 +436,10 @@ async fn host_model_info_uses_the_thread_catalog(source: SessionSource) -> Resul
|
||||
assert_eq!(selected.comp_hash.as_deref(), Some("thread-revision"));
|
||||
assert_eq!(selected.context_window, Some(32_000));
|
||||
|
||||
let model_config = options.config.to_models_manager_config();
|
||||
let thread = test.thread_manager.start_thread(options).await?.thread;
|
||||
assert_eq!(thread.selected_model_info().await, selected);
|
||||
let other_model = thread.model_info("after-update", &model_config).await.model;
|
||||
thread
|
||||
.update_thread_settings(ThreadSettingsOverrides {
|
||||
model: Some("after-update".into()),
|
||||
@@ -445,6 +447,7 @@ async fn host_model_info_uses_the_thread_catalog(source: SessionSource) -> Resul
|
||||
})
|
||||
.await?;
|
||||
let next = thread.selected_model_info().await;
|
||||
assert_eq!(next, other_model);
|
||||
assert_eq!(next.slug, "after-update");
|
||||
assert_eq!(next.comp_hash.as_deref(), Some("thread-revision"));
|
||||
Ok(())
|
||||
|
||||
@@ -68,8 +68,7 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
input.config.config_layer_stack.requirements(),
|
||||
);
|
||||
let scoring_enabled = policy.scoring_enabled();
|
||||
let sampler_config =
|
||||
super::startup::sampler_config(&input, self.thread_manager.upgrade()).await;
|
||||
let sampler_config = super::startup::sampler_config(&input).await;
|
||||
|
||||
if scoring_enabled && guardian_config.transcript.include_images {
|
||||
input
|
||||
|
||||
@@ -215,7 +215,7 @@ async fn idle_classifier_retains_the_owning_threads_workspace_binding() -> Resul
|
||||
.thread_store
|
||||
.get::<codex_model_provider::WorkspaceRoutingContext>()
|
||||
.expect("Core seeds the owning thread's routing context before extensions start");
|
||||
let classifier = crate::async_scorer::startup::sampler_config(&input, /*manager*/ None).await;
|
||||
let classifier = crate::async_scorer::startup::sampler_config(&input).await;
|
||||
classifier
|
||||
.provider
|
||||
.responses_api_provider(&owning_context)
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::SharedModelsManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::cyber_access_program::ApiKeyCyberAccessPrograms;
|
||||
use codex_extension_api::ThreadOriginator;
|
||||
@@ -107,21 +107,13 @@ pub(super) fn valid_implicit_decisions_key(key: &str) -> bool {
|
||||
!key.is_empty() && key.trim() == key && !codex_login::auth::is_fedramp_api_key(key)
|
||||
}
|
||||
|
||||
pub(super) async fn sampler_config(
|
||||
input: &ThreadStartInput<'_, Config>,
|
||||
manager: Option<Arc<ThreadManager>>,
|
||||
) -> LunaSamplerConfig {
|
||||
pub(super) async fn sampler_config(input: &ThreadStartInput<'_, Config>) -> LunaSamplerConfig {
|
||||
let inherit_parent_cyber_access_program =
|
||||
codex_core::RequiredCyberAccessPrograms::for_thread(input.thread_store).is_none();
|
||||
let luna_model = if let Some(thread_manager) = manager {
|
||||
let luna_model = if let Some(catalog) = input.thread_store.get::<SharedModelsManager>() {
|
||||
let mut model_config = input.config.to_models_manager_config();
|
||||
model_config.model_context_window = None;
|
||||
Some(
|
||||
thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info(MODEL, &model_config)
|
||||
.await,
|
||||
)
|
||||
Some(catalog.get_model_info(MODEL, &model_config).await)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
@@ -291,9 +291,8 @@ impl MemoryStartupContext {
|
||||
) -> StageOneRequestContext {
|
||||
let config_snapshot = self.thread.config_snapshot().await;
|
||||
let model_info = self
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info(model_name, &config.to_models_manager_config())
|
||||
.thread
|
||||
.model_info(model_name, &config.to_models_manager_config())
|
||||
.await;
|
||||
let reasoning_summary = config
|
||||
.model_reasoning_summary
|
||||
|
||||
Reference in New Issue
Block a user