From cd095e5efaec88fef940219ae25111d36b95ed70 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 9 Jun 2026 16:32:01 +0000 Subject: [PATCH] preserve model-derived instruction provenance --- .../external_agent_session_import.rs | 4 ++-- .../app-server/tests/suite/conversation_summary.rs | 2 +- codex-rs/app-server/tests/suite/v2/thread_read.rs | 2 +- .../app-server/tests/suite/v2/thread_unarchive.rs | 2 +- codex-rs/core/src/session/mod.rs | 14 +++++++++++--- codex-rs/core/src/session/session.rs | 14 ++++++++++++-- codex-rs/core/src/session/tests.rs | 7 +++++-- codex-rs/core/tests/suite/rollout_list_find.rs | 2 +- codex-rs/rollout/src/recorder.rs | 7 ++++--- codex-rs/rollout/src/recorder_tests.rs | 4 ++-- codex-rs/thread-store/src/in_memory.rs | 2 +- codex-rs/thread-store/src/local/mod.rs | 2 +- codex-rs/thread-store/src/types.rs | 7 +++++-- 13 files changed, 47 insertions(+), 22 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs b/codex-rs/app-server/src/request_processors/external_agent_session_import.rs index 4cec9bca16..e27c8e8d11 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs +++ b/codex-rs/app-server/src/request_processors/external_agent_session_import.rs @@ -182,12 +182,12 @@ impl ExternalAgentSessionImporter { parent_thread_id: None, source: source.clone(), thread_source: None, - base_instructions: BaseInstructions { + base_instructions: Some(BaseInstructions { text: config .base_instructions .clone() .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), - }, + }), dynamic_tools: Vec::new(), multi_agent_version: Some(MultiAgentVersion::V1), metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/suite/conversation_summary.rs b/codex-rs/app-server/tests/suite/conversation_summary.rs index 6ad9f1c633..58beaf6722 100644 --- a/codex-rs/app-server/tests/suite/conversation_summary.rs +++ b/codex-rs/app-server/tests/suite/conversation_summary.rs @@ -127,7 +127,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() -> parent_thread_id: None, source: SessionSource::Cli, thread_source: None, - base_instructions: BaseInstructions::default(), + base_instructions: Some(BaseInstructions::default()), dynamic_tools: Vec::new(), multi_agent_version: None, metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 272ecac79f..0ce1cb2e84 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -1361,7 +1361,7 @@ async fn seed_pathless_store_thread( parent_thread_id: None, source: ProtocolSessionSource::Cli, thread_source: None, - base_instructions: BaseInstructions::default(), + base_instructions: Some(BaseInstructions::default()), dynamic_tools: Vec::new(), multi_agent_version: None, metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs index 7267ab073a..5aa62e1ada 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs @@ -214,7 +214,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> { parent_thread_id: None, source: SessionSource::Cli, thread_source: None, - base_instructions: BaseInstructions::default(), + base_instructions: Some(BaseInstructions::default()), dynamic_tools: Vec::new(), multi_agent_version: None, metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index f3e506d40f..04cbb4eff6 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -216,6 +216,7 @@ pub(crate) use self::input_queue::TurnInput; pub(crate) use self::input_queue::TurnInputQueue; use self::review::spawn_review_thread; use self::session::AppServerClientMetadata; +use self::session::BaseInstructionsOrigin; use self::session::Session; use self::session::SessionConfiguration; pub(crate) use self::session::SessionSettingsUpdate; @@ -562,11 +563,17 @@ impl Codex { config .validate_multi_agent_v2_config() .map_err(|err| CodexErr::InvalidRequest(err.to_string()))?; - let base_instructions = config + let fixed_base_instructions = config .base_instructions .clone() - .or_else(|| conversation_history.get_base_instructions().map(|s| s.text)) - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)); + .or_else(|| conversation_history.get_base_instructions().map(|s| s.text)); + let (base_instructions, base_instructions_origin) = match fixed_base_instructions { + Some(base_instructions) => (base_instructions, BaseInstructionsOrigin::Fixed), + None => ( + model_info.get_model_instructions(config.personality), + BaseInstructionsOrigin::ModelDerived, + ), + }; // Dynamic tools are defined at thread start and persisted in rollout session metadata. let dynamic_tools = if dynamic_tools.is_empty() { @@ -629,6 +636,7 @@ impl Codex { let session = Box::pin(Session::new( session_configuration, + base_instructions_origin, config.clone(), installation_id, auth_manager.clone(), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index c6d0d9767e..9f9829b391 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -16,6 +16,12 @@ use codex_protocol::protocol::TurnEnvironmentSelections; use std::sync::OnceLock; use tokio::sync::Semaphore; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum BaseInstructionsOrigin { + Fixed, + ModelDerived, +} + /// Context for an initialized model agent /// /// A session has at most 1 running task at a time, and can be interrupted by user input. @@ -474,6 +480,7 @@ impl Session { )] pub(crate) async fn new( mut session_configuration: SessionConfiguration, + base_instructions_origin: BaseInstructionsOrigin, config: Arc, installation_id: String, auth_manager: Arc, @@ -548,8 +555,11 @@ impl Session { parent_thread_id, source: session_source, thread_source: session_configuration.thread_source, - base_instructions: BaseInstructions { - text: session_configuration.base_instructions.clone(), + base_instructions: match base_instructions_origin { + BaseInstructionsOrigin::Fixed => Some(BaseInstructions { + text: session_configuration.base_instructions.clone(), + }), + BaseInstructionsOrigin::ModelDerived => None, }, dynamic_tools: session_configuration.dynamic_tools.clone(), multi_agent_version: initial_multi_agent_version, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ef3f6dbfba..febd9f00b2 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3531,7 +3531,7 @@ async fn attach_thread_persistence(session: &mut Session) -> PathBuf { parent_thread_id: None, source: SessionSource::Exec, thread_source: None, - base_instructions: BaseInstructions::default(), + base_instructions: Some(BaseInstructions::default()), dynamic_tools: Vec::new(), multi_agent_version: None, metadata: ThreadPersistenceMetadata { @@ -4677,6 +4677,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { )); let result = Session::new( session_configuration, + BaseInstructionsOrigin::Fixed, Arc::clone(&config), "11111111-1111-4111-8111-111111111111".to_string(), auth_manager, @@ -5017,6 +5018,7 @@ async fn make_session_with_config_and_rx( let session = Session::new( session_configuration, + BaseInstructionsOrigin::Fixed, Arc::clone(&config), "11111111-1111-4111-8111-111111111111".to_string(), auth_manager, @@ -5118,6 +5120,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( let session = Session::new( session_configuration, + BaseInstructionsOrigin::Fixed, Arc::clone(&config), "11111111-1111-4111-8111-111111111111".to_string(), auth_manager, @@ -6366,7 +6369,7 @@ async fn shutdown_complete_does_not_append_to_thread_store_after_shutdown() { parent_thread_id: None, source: SessionSource::Exec, thread_source: None, - base_instructions: BaseInstructions::default(), + base_instructions: Some(BaseInstructions::default()), dynamic_tools: Vec::new(), multi_agent_version: None, metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/core/tests/suite/rollout_list_find.rs b/codex-rs/core/tests/suite/rollout_list_find.rs index 4ff35449cb..2a1d1e40ed 100644 --- a/codex-rs/core/tests/suite/rollout_list_find.rs +++ b/codex-rs/core/tests/suite/rollout_list_find.rs @@ -187,7 +187,7 @@ async fn find_locates_rollout_file_written_by_recorder() -> std::io::Result<()> /*parent_thread_id*/ None, SessionSource::Exec, /*thread_source*/ None, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), ), ) diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index d102d5c540..fd801206a9 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -86,7 +86,8 @@ pub enum RolloutRecorderParams { parent_thread_id: Option, source: SessionSource, thread_source: Option, - base_instructions: BaseInstructions, + /// Fixed instructions to persist; model-derived instructions remain unresolved. + base_instructions: Option, dynamic_tools: Vec, multi_agent_version: Option, }, @@ -163,7 +164,7 @@ impl RolloutRecorderParams { parent_thread_id: Option, source: SessionSource, thread_source: Option, - base_instructions: BaseInstructions, + base_instructions: Option, dynamic_tools: Vec, ) -> Self { Self::Create { @@ -707,7 +708,7 @@ impl RolloutRecorder { source, thread_source, model_provider: Some(config.model_provider_id().to_string()), - base_instructions: Some(base_instructions), + base_instructions, dynamic_tools: if dynamic_tools.is_empty() { None } else { diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 07029abe37..7e7433b1e8 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -374,7 +374,7 @@ async fn recorder_materializes_on_flush_with_pending_items() -> std::io::Result< /*parent_thread_id*/ None, SessionSource::Exec, /*thread_source*/ None, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), ), ) @@ -455,7 +455,7 @@ async fn persist_reports_filesystem_error_and_retries_buffered_items() -> std::i /*parent_thread_id*/ None, SessionSource::Exec, /*thread_source*/ None, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), ), ) diff --git a/codex-rs/thread-store/src/in_memory.rs b/codex-rs/thread-store/src/in_memory.rs index edc308a905..28d5ae4148 100644 --- a/codex-rs/thread-store/src/in_memory.rs +++ b/codex-rs/thread-store/src/in_memory.rs @@ -178,7 +178,7 @@ impl ThreadStore for InMemoryThreadStore { source: params.source.clone(), thread_source: params.thread_source, model_provider: Some(params.metadata.model_provider.clone()), - base_instructions: Some(params.base_instructions.clone()), + base_instructions: params.base_instructions.clone(), dynamic_tools: (!params.dynamic_tools.is_empty()).then(|| params.dynamic_tools.clone()), memory_mode: matches!(params.metadata.memory_mode, ThreadMemoryMode::Disabled) .then_some("disabled".to_string()), diff --git a/codex-rs/thread-store/src/local/mod.rs b/codex-rs/thread-store/src/local/mod.rs index cef4496a02..606d231716 100644 --- a/codex-rs/thread-store/src/local/mod.rs +++ b/codex-rs/thread-store/src/local/mod.rs @@ -1018,7 +1018,7 @@ mod tests { parent_thread_id: None, source: SessionSource::Exec, thread_source: None, - base_instructions: BaseInstructions::default(), + base_instructions: Some(BaseInstructions::default()), dynamic_tools: Vec::new(), multi_agent_version: None, metadata: thread_metadata(), diff --git a/codex-rs/thread-store/src/types.rs b/codex-rs/thread-store/src/types.rs index 1f18adf081..db7a774355 100644 --- a/codex-rs/thread-store/src/types.rs +++ b/codex-rs/thread-store/src/types.rs @@ -75,8 +75,11 @@ pub struct CreateThreadParams { pub source: SessionSource, /// Optional analytics source classification for this thread. pub thread_source: Option, - /// Base instructions persisted in session metadata. - pub base_instructions: BaseInstructions, + /// Fixed base instructions persisted in session metadata. + /// + /// `None` leaves model-derived instructions unresolved so the current model catalog can rebuild + /// them when the thread is resumed. + pub base_instructions: Option, /// Dynamic tools available to the thread at startup. pub dynamic_tools: Vec, /// Multi-agent runtime selected when the thread was created.