feat: use provider defaults for memory models (#27129)

## Why

Memory startup used hardcoded OpenAI model slugs for extraction and
consolidation. That works for the default OpenAI-compatible path, but
provider-specific backends can require different model identifiers. In
particular, Amazon Bedrock should use its Bedrock model ID for these
background memory requests instead of the OpenAI `gpt-5.4-mini` /
`gpt-5.4` slugs.

## What Changed

- Added provider-owned preferred memory model methods alongside
`approval_review_preferred_model`.
- Updated memory extraction and consolidation to resolve their default
model through the active `ModelProvider`.
- Added Amazon Bedrock overrides so both memory stages use
`openai.gpt-5.4` through Bedrock’s provider-specific model ID.
- Kept explicit `memories.extract_model` and
`memories.consolidation_model` config overrides taking precedence.
- Added startup coverage for default OpenAI and Bedrock memory model
selection.

#closes #26288
This commit is contained in:
Celia Chen
2026-06-09 16:49:09 -07:00
committed by GitHub
Unverified
parent fb8f1ea0d5
commit 51fc4b0559
9 changed files with 413 additions and 13 deletions
+2
View File
@@ -3342,6 +3342,8 @@ dependencies = [
"codex-features",
"codex-git-utils",
"codex-login",
"codex-model-provider",
"codex-model-provider-info",
"codex-models-manager",
"codex-otel",
"codex-protocol",
+2
View File
@@ -21,6 +21,7 @@ codex-config = { workspace = true }
codex-features = { workspace = true }
codex-git-utils = { workspace = true }
codex-login = { workspace = true }
codex-model-provider = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-rollout = { workspace = true }
@@ -39,6 +40,7 @@ tracing = { workspace = true, features = ["log"] }
uuid = { workspace = true, features = ["v4", "v5"] }
[dev-dependencies]
codex-model-provider-info = { workspace = true }
codex-models-manager = { workspace = true }
core_test_support = { workspace = true }
pretty_assertions = { workspace = true }
-2
View File
@@ -76,7 +76,6 @@ signal to remove stale memories derived only from those resources.
}
mod stage_one {
pub(super) const MODEL: &str = "gpt-5.4-mini";
pub(super) const REASONING_EFFORT: codex_protocol::openai_models::ReasoningEffort =
codex_protocol::openai_models::ReasoningEffort::Low;
pub(super) const CONCURRENCY_LIMIT: usize = 8;
@@ -101,7 +100,6 @@ mod stage_one {
}
mod stage_two {
pub(super) const MODEL: &str = "gpt-5.4";
pub(super) const REASONING_EFFORT: codex_protocol::openai_models::ReasoningEffort =
codex_protocol::openai_models::ReasoningEffort::Medium;
pub(super) const JOB_LEASE_SECONDS: i64 = 3_600;
+6 -5
View File
@@ -190,11 +190,12 @@ async fn build_request_context(
context: &MemoryStartupContext,
config: &Config,
) -> StageOneRequestContext {
let model_name = config
.memories
.extract_model
.clone()
.unwrap_or(crate::stage_one::MODEL.to_string());
let model_name = config.memories.extract_model.clone().unwrap_or_else(|| {
context
.provider()
.memory_extraction_preferred_model()
.to_string()
});
context
.stage_one_request_context(config, &model_name, crate::stage_one::REASONING_EFFORT)
.await
+4 -3
View File
@@ -16,6 +16,7 @@ use crate::workspace::write_workspace_diff;
use codex_config::Constrained;
use codex_core::config::Config;
use codex_features::Feature;
use codex_model_provider::ModelProvider;
use codex_protocol::ThreadId;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::AskForApproval;
@@ -76,7 +77,7 @@ pub async fn run(context: Arc<MemoryStartupContext>, config: Arc<Config>) {
}
// 3. Build the locked-down config used by the consolidation agent.
let Some(agent_config) = agent::get_config(config.as_ref()) else {
let Some(agent_config) = agent::get_config(config.as_ref(), context.provider()) else {
// If we can't get the config, we can't consolidate.
tracing::error!("failed to get agent config");
job::failed(
@@ -297,7 +298,7 @@ mod agent {
use super::*;
use tracing::warn;
pub(super) fn get_config(config: &Config) -> Option<Config> {
pub(super) fn get_config(config: &Config, provider: &dyn ModelProvider) -> Option<Config> {
let root = memory_root(&config.codex_home);
let mut agent_config = config.clone();
@@ -339,7 +340,7 @@ mod agent {
.memories
.consolidation_model
.clone()
.unwrap_or(crate::stage_two::MODEL.to_string()),
.unwrap_or_else(|| provider.memory_consolidation_preferred_model().to_string()),
);
agent_config.model_reasoning_effort = Some(crate::stage_two::REASONING_EFFORT);
+54
View File
@@ -13,6 +13,9 @@ use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_login::auth_env_telemetry::collect_auth_env_telemetry;
use codex_login::default_client::originator;
use codex_model_provider::ModelProvider;
use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider;
use codex_otel::SessionTelemetry;
use codex_otel::TelemetryAuthMode;
use codex_protocol::SessionId;
@@ -68,6 +71,7 @@ pub(crate) struct MemoryStartupContext {
thread: Arc<CodexThread>,
thread_manager: Arc<ThreadManager>,
auth_manager: Arc<AuthManager>,
provider: SharedModelProvider,
session_telemetry: SessionTelemetry,
}
@@ -79,6 +83,51 @@ impl MemoryStartupContext {
thread: Arc<CodexThread>,
config: &Config,
source: SessionSource,
) -> Self {
let provider = create_model_provider(
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
);
Self::new_with_provider(
thread_manager,
auth_manager,
thread_id,
thread,
config,
source,
provider,
)
}
#[cfg(test)]
pub(crate) fn new_for_testing(
thread_manager: Arc<ThreadManager>,
auth_manager: Arc<AuthManager>,
thread_id: ThreadId,
thread: Arc<CodexThread>,
config: &Config,
source: SessionSource,
provider: SharedModelProvider,
) -> Self {
Self::new_with_provider(
thread_manager,
auth_manager,
thread_id,
thread,
config,
source,
provider,
)
}
fn new_with_provider(
thread_manager: Arc<ThreadManager>,
auth_manager: Arc<AuthManager>,
thread_id: ThreadId,
thread: Arc<CodexThread>,
config: &Config,
source: SessionSource,
provider: SharedModelProvider,
) -> Self {
let auth = auth_manager.auth_cached();
let auth = auth.as_ref();
@@ -109,6 +158,7 @@ impl MemoryStartupContext {
thread,
thread_manager,
auth_manager,
provider,
session_telemetry,
}
}
@@ -121,6 +171,10 @@ impl MemoryStartupContext {
self.thread.state_db()
}
pub(crate) fn provider(&self) -> &dyn ModelProvider {
self.provider.as_ref()
}
pub(crate) fn counter(&self, name: &str, inc: i64, tags: &[(&str, &str)]) {
self.session_telemetry.counter(name, inc, tags);
}
+315 -3
View File
@@ -1,12 +1,30 @@
use crate::extensions::seed_extension_instructions;
use crate::memory_root;
use crate::phase1;
use crate::phase2;
use crate::runtime::MemoryStartupContext;
use crate::start_memories_startup_task;
use codex_config::types::MemoriesConfig;
use codex_features::Feature;
use codex_git_utils::diff_since_latest_init;
use codex_git_utils::reset_git_repository;
use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_model_provider::ModelProvider;
use codex_model_provider::ProviderAccountResult;
use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider;
use codex_model_provider_info::ModelProviderInfo;
use codex_protocol::ThreadId;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::SessionSource;
use core_test_support::responses::ResponseMock;
use core_test_support::responses::ResponsesRequest;
@@ -20,7 +38,10 @@ use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use std::future::Future;
use std::path::Path;
use std::path::PathBuf;
use std::pin::Pin;
use std::sync::Arc;
use tempfile::TempDir;
use tokio::time::Duration;
@@ -329,18 +350,186 @@ async fn memories_startup_phase1_uses_live_thread_service_tier_and_detached_meta
Ok(())
}
#[tokio::test]
async fn memories_startup_phase1_provider_default_drives_request_model() -> anyhow::Result<()> {
let server = start_mock_server().await;
let home = Arc::new(TempDir::new()?);
let request =
run_memory_phase_one_model_request_test(&server, home, startup_test_memories_config())
.await?;
assert_eq!(
request.body_json()["model"].as_str(),
Some(MOCK_PROVIDER_PHASE_ONE_MODEL)
);
Ok(())
}
#[tokio::test]
async fn memories_startup_phase2_provider_default_drives_request_model() -> anyhow::Result<()> {
let server = start_mock_server().await;
let home = Arc::new(TempDir::new()?);
let request =
run_memory_phase_two_model_request_test(&server, home, startup_test_memories_config())
.await?;
assert_eq!(
request.body_json()["model"].as_str(),
Some(MOCK_PROVIDER_PHASE_TWO_MODEL)
);
Ok(())
}
#[tokio::test]
async fn memories_startup_phase1_explicit_model_override_drives_request_model() -> anyhow::Result<()>
{
let server = start_mock_server().await;
let home = Arc::new(TempDir::new()?);
let mut memories = startup_test_memories_config();
memories.extract_model = Some("override.phase-one".to_string());
let request = run_memory_phase_one_model_request_test(&server, home, memories).await?;
assert_eq!(
request.body_json()["model"].as_str(),
Some("override.phase-one")
);
Ok(())
}
#[tokio::test]
async fn memories_startup_phase2_explicit_model_override_drives_request_model() -> anyhow::Result<()>
{
let server = start_mock_server().await;
let home = Arc::new(TempDir::new()?);
let mut memories = startup_test_memories_config();
memories.consolidation_model = Some("override.phase-two".to_string());
let request = run_memory_phase_two_model_request_test(&server, home, memories).await?;
assert_eq!(
request.body_json()["model"].as_str(),
Some("override.phase-two")
);
Ok(())
}
async fn run_memory_phase_one_model_request_test(
server: &wiremock::MockServer,
home: Arc<TempDir>,
memories: MemoriesConfig,
) -> anyhow::Result<ResponsesRequest> {
let test = build_test_codex_with_memories_config(server, Arc::clone(&home), memories).await?;
let provider = Arc::new(MockMemoryModelProvider::new(
test.config.model_provider.clone(),
Some(test.thread_manager.auth_manager()),
));
let db = test
.codex
.state_db()
.ok_or_else(|| anyhow::anyhow!("state db should be enabled for memory startup test"))?;
seed_stage1_candidate(
db.as_ref(),
home.path(),
chrono::Utc::now() - chrono::Duration::hours(2),
"startup-models",
)
.await?;
let response = mount_sse_once(
server,
sse(vec![
ev_response_created("resp-phase1"),
ev_assistant_message(
"msg-phase1",
r#"{"raw_memory":"raw memory","rollout_summary":"rollout summary","rollout_slug":"startup-models"}"#,
),
ev_completed("resp-phase1"),
]),
)
.await;
let (context, config) = memory_startup_context_with_provider(&test, provider).await;
phase1::run(context, config).await;
let request = wait_for_single_request(&response).await;
shutdown_test_codex(&test).await?;
Ok(request)
}
async fn run_memory_phase_two_model_request_test(
server: &wiremock::MockServer,
home: Arc<TempDir>,
memories: MemoriesConfig,
) -> anyhow::Result<ResponsesRequest> {
let test = build_test_codex_with_memories_config(server, home.clone(), memories).await?;
let provider = Arc::new(MockMemoryModelProvider::new(
test.config.model_provider.clone(),
Some(test.thread_manager.auth_manager()),
));
let db = test
.codex
.state_db()
.ok_or_else(|| anyhow::anyhow!("state db should be enabled for memory startup test"))?;
seed_stage1_output(
db.as_ref(),
home.path(),
chrono::Utc::now(),
"raw memory for phase two",
"rollout summary for phase two",
"startup-models-phase-two",
)
.await?;
let response = mount_sse_once(
server,
sse(vec![
ev_response_created("resp-phase2"),
ev_assistant_message("msg-phase2", "phase2 complete"),
ev_completed("resp-phase2"),
]),
)
.await;
let (context, config) = memory_startup_context_with_provider(&test, provider).await;
let root = memory_root(&config.codex_home);
tokio::fs::create_dir_all(&root).await?;
seed_extension_instructions(&root).await?;
phase2::run(context, config).await;
let request = wait_for_single_request(&response).await;
wait_for_phase2_workspace_reset(&home.path().join("memories")).await?;
shutdown_test_codex(&test).await?;
Ok(request)
}
fn startup_test_memories_config() -> MemoriesConfig {
MemoriesConfig {
max_raw_memories_for_consolidation: 1,
min_rollout_idle_hours: 0,
..MemoriesConfig::default()
}
}
async fn build_test_codex(
server: &wiremock::MockServer,
home: Arc<TempDir>,
) -> anyhow::Result<TestCodex> {
build_test_codex_with_memories_config(server, home, startup_test_memories_config()).await
}
async fn build_test_codex_with_memories_config(
server: &wiremock::MockServer,
home: Arc<TempDir>,
memories: MemoriesConfig,
) -> anyhow::Result<TestCodex> {
test_codex()
.with_home(home)
.with_config(|config| {
.with_config(move |config| {
config
.features
.enable(Feature::Sqlite)
.expect("test config should allow feature update");
config.memories.max_raw_memories_for_consolidation = 1;
config.memories = memories;
})
.build(server)
.await
@@ -370,6 +559,88 @@ async fn trigger_memories_startup(test: &TestCodex) {
);
}
async fn memory_startup_context_with_provider(
test: &TestCodex,
provider: SharedModelProvider,
) -> (Arc<MemoryStartupContext>, Arc<codex_core::config::Config>) {
let config_snapshot = test.codex.config_snapshot().await;
let mut config = test.config.clone();
config
.features
.enable(Feature::MemoryTool)
.expect("test config should allow feature update");
let config = Arc::new(config);
let context = Arc::new(MemoryStartupContext::new_for_testing(
Arc::clone(&test.thread_manager),
test.thread_manager.auth_manager(),
test.session_configured.thread_id,
Arc::clone(&test.codex),
config.as_ref(),
config_snapshot.session_source,
provider,
));
(context, config)
}
const MOCK_PROVIDER_PHASE_ONE_MODEL: &str = "mock.phase-one";
const MOCK_PROVIDER_PHASE_TWO_MODEL: &str = "mock.phase-two";
#[derive(Debug)]
struct MockMemoryModelProvider {
delegate: SharedModelProvider,
}
impl MockMemoryModelProvider {
fn new(info: ModelProviderInfo, auth_manager: Option<Arc<AuthManager>>) -> Self {
Self {
delegate: create_model_provider(info, auth_manager),
}
}
}
impl ModelProvider for MockMemoryModelProvider {
fn info(&self) -> &ModelProviderInfo {
self.delegate.info()
}
fn memory_extraction_preferred_model(&self) -> &'static str {
MOCK_PROVIDER_PHASE_ONE_MODEL
}
fn memory_consolidation_preferred_model(&self) -> &'static str {
MOCK_PROVIDER_PHASE_TWO_MODEL
}
fn auth_manager(&self) -> Option<Arc<AuthManager>> {
self.delegate.auth_manager()
}
fn auth<'life0, 'async_trait>(
&'life0 self,
) -> Pin<Box<dyn Future<Output = Option<CodexAuth>> + Send + 'async_trait>>
where
'life0: 'async_trait,
Self: 'async_trait,
{
let delegate = Arc::clone(&self.delegate);
Box::pin(async move { delegate.auth().await })
}
fn account_state(&self) -> ProviderAccountResult {
self.delegate.account_state()
}
fn models_manager(
&self,
codex_home: PathBuf,
config_model_catalog: Option<ModelsResponse>,
) -> codex_models_manager::manager::SharedModelsManager {
self.delegate
.models_manager(codex_home, config_model_catalog)
}
}
async fn seed_stage1_output(
db: &codex_state::StateRuntime,
codex_home: &Path,
@@ -404,6 +675,46 @@ async fn seed_stage1_output(
Ok(thread_id)
}
async fn seed_stage1_candidate(
db: &codex_state::StateRuntime,
codex_home: &Path,
updated_at: chrono::DateTime<chrono::Utc>,
rollout_slug: &str,
) -> anyhow::Result<ThreadId> {
let thread_id = ThreadId::new();
let rollout_path = codex_home.join(format!("rollout-{thread_id}.jsonl"));
let line = RolloutLine {
timestamp: updated_at.to_rfc3339(),
item: RolloutItem::ResponseItem(ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: "remember this startup test conversation".to_string(),
}],
phase: None,
}),
};
let jsonl = serde_json::to_string(&line)?;
tokio::fs::write(&rollout_path, format!("{jsonl}\n")).await?;
let mut metadata_builder = codex_state::ThreadMetadataBuilder::new(
thread_id,
rollout_path,
updated_at,
SessionSource::Cli,
);
metadata_builder.cwd = codex_home.join(format!("workspace-{rollout_slug}"));
metadata_builder.model_provider = Some("test-provider".to_string());
metadata_builder.git_branch = Some(format!("branch-{rollout_slug}"));
let mut metadata = metadata_builder.build("test-provider");
metadata.preview = Some("remember this startup test conversation".to_string());
metadata.first_user_message = metadata.preview.clone();
db.upsert_thread(&metadata).await?;
db.set_thread_memory_mode(thread_id, "enabled").await?;
Ok(thread_id)
}
async fn wait_for_single_request(mock: &ResponseMock) -> ResponsesRequest {
wait_for_request(mock, /*expected_count*/ 1).await.remove(0)
}
@@ -447,7 +758,8 @@ async fn wait_for_request(mock: &ResponseMock, expected_count: usize) -> Vec<Res
}
assert!(
Instant::now() < deadline,
"timed out waiting for {expected_count} phase2 requests"
"timed out waiting for {expected_count} responses requests, got {}",
requests.len()
);
tokio::time::sleep(Duration::from_millis(50)).await;
}
@@ -68,6 +68,14 @@ impl ModelProvider for AmazonBedrockModelProvider {
AMAZON_BEDROCK_GPT_5_4_MODEL_ID
}
fn memory_extraction_preferred_model(&self) -> &'static str {
AMAZON_BEDROCK_GPT_5_4_MODEL_ID
}
fn memory_consolidation_preferred_model(&self) -> &'static str {
AMAZON_BEDROCK_GPT_5_4_MODEL_ID
}
fn auth_manager(&self) -> Option<Arc<AuthManager>> {
None
}
+22
View File
@@ -74,6 +74,14 @@ pub type ProviderAccountResult = std::result::Result<ProviderAccountState, Provi
/// require a backend-specific model ID.
pub const DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL: &str = "codex-auto-review";
/// Default model used for memory extraction when a provider does not require a
/// backend-specific model ID.
pub const DEFAULT_MEMORY_EXTRACTION_PREFERRED_MODEL: &str = "gpt-5.4-mini";
/// Default model used for memory consolidation when a provider does not require
/// a backend-specific model ID.
pub const DEFAULT_MEMORY_CONSOLIDATION_PREFERRED_MODEL: &str = "gpt-5.4";
/// Runtime provider abstraction used by model execution.
///
/// Implementations own provider-specific behavior for a model backend. The
@@ -96,6 +104,20 @@ pub trait ModelProvider: fmt::Debug + Send + Sync {
DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL
}
/// Returns the preferred model used for memory extraction.
///
/// Providers that require backend-specific model IDs should override this.
fn memory_extraction_preferred_model(&self) -> &'static str {
DEFAULT_MEMORY_EXTRACTION_PREFERRED_MODEL
}
/// Returns the preferred model used for memory consolidation.
///
/// Providers that require backend-specific model IDs should override this.
fn memory_consolidation_preferred_model(&self) -> &'static str {
DEFAULT_MEMORY_CONSOLIDATION_PREFERRED_MODEL
}
/// Returns whether requests made through this provider should include attestation.
fn supports_attestation(&self) -> bool {
false