diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts index a7ba31180..952da565a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts @@ -27,7 +27,7 @@ model?: string | null, modelProvider?: string | null, serviceTier?: ServiceTier * Override where approval requests are routed for review on this thread * and subsequent turns. */ -approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, ephemeral?: boolean, /** +approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null | null, developerInstructions?: string | null | null, ephemeral?: boolean, /** * If true, persist additional rollout EventMsg variants required to * reconstruct a richer thread history on subsequent resume/fork/read. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts index 770344de8..fcde0ae50 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts @@ -36,7 +36,7 @@ model?: string | null, modelProvider?: string | null, serviceTier?: ServiceTier * Override where approval requests are routed for review on this thread * and subsequent turns. */ -approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, personality?: Personality | null, /** +approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null | null, developerInstructions?: string | null | null, personality?: Personality | null, /** * If true, persist additional rollout EventMsg variants required to * reconstruct a richer thread history on subsequent resume/fork/read. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts index 61f501ad6..c7b27823f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts @@ -12,7 +12,7 @@ export type ThreadStartParams = {model?: string | null, modelProvider?: string | * Override where approval requests are routed for review on this thread * and subsequent turns. */ -approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, serviceName?: string | null, baseInstructions?: string | null, developerInstructions?: string | null, personality?: Personality | null, ephemeral?: boolean | null, /** +approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, serviceName?: string | null, baseInstructions?: string | null | null, developerInstructions?: string | null | null, personality?: Personality | null, ephemeral?: boolean | null, /** * If true, opt into emitting raw Responses API items on the event stream. * This is for internal use only (e.g. Codex Cloud). */ diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 61b7db45e..3562d9cda 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -2603,10 +2603,22 @@ pub struct ThreadStartParams { pub config: Option>, #[ts(optional = nullable)] pub service_name: Option, + #[serde( + default, + deserialize_with = "super::serde_helpers::deserialize_double_option", + serialize_with = "super::serde_helpers::serialize_double_option", + skip_serializing_if = "Option::is_none" + )] #[ts(optional = nullable)] - pub base_instructions: Option, + pub base_instructions: Option>, + #[serde( + default, + deserialize_with = "super::serde_helpers::deserialize_double_option", + serialize_with = "super::serde_helpers::serialize_double_option", + skip_serializing_if = "Option::is_none" + )] #[ts(optional = nullable)] - pub developer_instructions: Option, + pub developer_instructions: Option>, #[ts(optional = nullable)] pub personality: Option, #[ts(optional = nullable)] @@ -2721,10 +2733,22 @@ pub struct ThreadResumeParams { pub sandbox: Option, #[ts(optional = nullable)] pub config: Option>, + #[serde( + default, + deserialize_with = "super::serde_helpers::deserialize_double_option", + serialize_with = "super::serde_helpers::serialize_double_option", + skip_serializing_if = "Option::is_none" + )] #[ts(optional = nullable)] - pub base_instructions: Option, + pub base_instructions: Option>, + #[serde( + default, + deserialize_with = "super::serde_helpers::deserialize_double_option", + serialize_with = "super::serde_helpers::serialize_double_option", + skip_serializing_if = "Option::is_none" + )] #[ts(optional = nullable)] - pub developer_instructions: Option, + pub developer_instructions: Option>, #[ts(optional = nullable)] pub personality: Option, /// If true, persist additional rollout EventMsg variants required to @@ -2798,10 +2822,22 @@ pub struct ThreadForkParams { pub sandbox: Option, #[ts(optional = nullable)] pub config: Option>, + #[serde( + default, + deserialize_with = "super::serde_helpers::deserialize_double_option", + serialize_with = "super::serde_helpers::serialize_double_option", + skip_serializing_if = "Option::is_none" + )] #[ts(optional = nullable)] - pub base_instructions: Option, + pub base_instructions: Option>, + #[serde( + default, + deserialize_with = "super::serde_helpers::deserialize_double_option", + serialize_with = "super::serde_helpers::serialize_double_option", + skip_serializing_if = "Option::is_none" + )] #[ts(optional = nullable)] - pub developer_instructions: Option, + pub developer_instructions: Option>, #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub ephemeral: bool, /// If true, persist additional rollout EventMsg variants required to @@ -8325,6 +8361,35 @@ mod tests { assert_eq!(serialized_without_override.get("serviceTier"), None); } + #[test] + fn thread_start_params_preserve_explicit_null_instructions() { + let params: ThreadStartParams = serde_json::from_value(json!({ + "baseInstructions": null, + "developerInstructions": null, + })) + .expect("params should deserialize"); + assert_eq!(params.base_instructions, Some(None)); + assert_eq!(params.developer_instructions, Some(None)); + + let serialized = serde_json::to_value(¶ms).expect("params should serialize"); + assert_eq!( + serialized.get("baseInstructions"), + Some(&serde_json::Value::Null) + ); + assert_eq!( + serialized.get("developerInstructions"), + Some(&serde_json::Value::Null) + ); + + let serialized_without_override = + serde_json::to_value(ThreadStartParams::default()).expect("params should serialize"); + assert_eq!(serialized_without_override.get("baseInstructions"), None); + assert_eq!( + serialized_without_override.get("developerInstructions"), + None + ); + } + #[test] fn turn_start_params_preserve_explicit_null_service_tier() { let params: TurnStartParams = serde_json::from_value(json!({ diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 920fcbece..91641898d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2473,8 +2473,8 @@ impl CodexMessageProcessor { approval_policy: Option, approvals_reviewer: Option, sandbox: Option, - base_instructions: Option, - developer_instructions: Option, + base_instructions: Option>, + developer_instructions: Option>, personality: Option, ) -> ConfigOverrides { ConfigOverrides { @@ -4363,6 +4363,13 @@ impl CodexMessageProcessor { developer_instructions, /*personality*/ None, ); + if typesafe_overrides.base_instructions.is_none() + && let Ok(history) = RolloutRecorder::get_rollout_history(&rollout_path).await + && let Some(base_instructions) = history.get_base_instructions() + { + typesafe_overrides.base_instructions = + Some(base_instructions.map(|base_instructions| base_instructions.text)); + } typesafe_overrides.ephemeral = ephemeral.then_some(true); // Derive a Config using the same logic as new conversation, honoring overrides if provided. let cloud_requirements = self.current_cloud_requirements(); diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 9907fc4b1..b4e80d781 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -26,6 +26,8 @@ use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput; use codex_config::types::AuthCredentialsStoreMode; use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; +use core_test_support::responses; +use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -183,6 +185,153 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_fork_honors_explicit_null_thread_instructions() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("msg-1", "Done"), + responses::ev_completed("resp-1"), + ]); + let response_mock = + responses::mount_sse_sequence(&server, vec![body.clone(), body.clone(), body]).await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let conversation_id = create_fake_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Saved user message", + Some("mock_provider"), + /*git_info*/ None, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let disabled_instruction_config = json!({ + "include_permissions_instructions": false, + "include_apps_instructions": false, + "include_environment_context": false, + "features.apps": false, + "features.plugins": false, + "features.codex_hooks": false, + "skills.bundled.enabled": false, + }); + + let fork_params = [ + ( + json!({ + "threadId": conversation_id.clone(), + "config": disabled_instruction_config.clone(), + }), + /*expect_instructions*/ true, + ), + ( + json!({ + "threadId": conversation_id.clone(), + "config": disabled_instruction_config.clone(), + "baseInstructions": null, + "developerInstructions": null, + }), + /*expect_instructions*/ false, + ), + ]; + + let mut forked_thread_ids = Vec::new(); + for (params, _expect_instructions) in fork_params { + let fork_id = mcp.send_raw_request("thread/fork", Some(params)).await?; + let fork_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(fork_id)), + ) + .await??; + let ThreadForkResponse { thread, .. } = to_response::(fork_resp)?; + forked_thread_ids.push(thread.id.clone()); + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![UserInput::Text { + text: "continue".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + let _: TurnStartResponse = to_response::(turn_resp)?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + } + + let refork_id = mcp + .send_raw_request( + "thread/fork", + Some(json!({ + "threadId": forked_thread_ids[1].clone(), + "config": disabled_instruction_config.clone(), + })), + ) + .await?; + let refork_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(refork_id)), + ) + .await??; + let ThreadForkResponse { thread, .. } = to_response::(refork_resp)?; + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![UserInput::Text { + text: "continue again".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + let _: TurnStartResponse = to_response::(turn_resp)?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let requests = response_mock.requests(); + assert_eq!(requests.len(), 3); + for (request, expect_instructions) in requests.into_iter().zip([true, false, false]) { + let payload = request.body_json(); + assert_eq!( + payload.get("instructions").is_some(), + expect_instructions, + "unexpected instructions field in payload: {payload:?}" + ); + let developer_texts = request.message_input_texts("developer"); + assert!( + developer_texts.iter().all(|text| !text.is_empty()), + "did not expect empty developer instruction messages: {developer_texts:?}" + ); + } + + Ok(()) +} + #[tokio::test] async fn thread_fork_tracks_thread_initialized_analytics() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 59591dd0d..e8f3e1df2 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -152,6 +152,103 @@ async fn turn_start_sends_originator_header() -> Result<()> { Ok(()) } +#[tokio::test] +async fn turn_start_honors_explicit_null_thread_instructions() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("msg-1", "Done"), + responses::ev_completed("resp-1"), + ]); + let response_mock = responses::mount_sse_sequence(&server, vec![body.clone(), body]).await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri(), "never", &BTreeMap::new())?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let disabled_instruction_config = json!({ + "include_permissions_instructions": false, + "include_apps_instructions": false, + "include_environment_context": false, + "features.apps": false, + "features.plugins": false, + "features.codex_hooks": false, + "skills.bundled.enabled": false, + }); + + let thread_start_params = [ + ( + json!({ + "model": "mock-model", + "config": disabled_instruction_config.clone(), + }), + /*expect_instructions*/ true, + ), + ( + json!({ + "model": "mock-model", + "config": disabled_instruction_config.clone(), + "baseInstructions": null, + "developerInstructions": null, + }), + /*expect_instructions*/ false, + ), + ]; + + for (params, _expect_instructions) in thread_start_params { + let thread_req = mcp.send_raw_request("thread/start", Some(params)).await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + } + + let requests = response_mock.requests(); + assert_eq!(requests.len(), 2); + for (request, expect_instructions) in requests.into_iter().zip([true, false]) { + let payload = request.body_json(); + assert_eq!( + payload.get("instructions").is_some(), + expect_instructions, + "unexpected instructions field in payload: {payload:?}" + ); + let developer_texts = request.message_input_texts("developer"); + assert!( + developer_texts.iter().all(|text| !text.is_empty()), + "did not expect empty developer instruction messages: {developer_texts:?}" + ); + } + + Ok(()) +} + #[tokio::test] async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> { let responses = vec![create_final_assistant_message_sse_response("Done")?]; diff --git a/codex-rs/codex-api/src/common.rs b/codex-rs/codex-api/src/common.rs index 39fb976e6..a2a2c1d47 100644 --- a/codex-rs/codex-api/src/common.rs +++ b/codex-rs/codex-api/src/common.rs @@ -153,7 +153,8 @@ impl From for OpenAiVerbosity { #[derive(Debug, Serialize, Clone, PartialEq)] pub struct ResponsesApiRequest { pub model: String, - pub instructions: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub instructions: Option, pub input: Vec, pub tools: Vec, pub tool_choice: String, @@ -196,7 +197,8 @@ impl From<&ResponsesApiRequest> for ResponseCreateWsRequest { #[derive(Debug, Serialize)] pub struct ResponseCreateWsRequest { pub model: String, - pub instructions: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub instructions: Option, #[serde(skip_serializing_if = "Option::is_none")] pub previous_response_id: Option, pub input: Vec, diff --git a/codex-rs/codex-api/tests/clients.rs b/codex-rs/codex-api/tests/clients.rs index ba0f41678..2f3bc17e2 100644 --- a/codex-rs/codex-api/tests/clients.rs +++ b/codex-rs/codex-api/tests/clients.rs @@ -266,7 +266,7 @@ async fn streaming_client_retries_on_transport_error() -> Result<()> { let request = ResponsesApiRequest { model: "gpt-test".into(), - instructions: "Say hi".into(), + instructions: Some("Say hi".into()), input: Vec::new(), tools: Vec::new(), tool_choice: "auto".into(), @@ -302,7 +302,7 @@ async fn azure_default_store_attaches_ids_and_headers() -> Result<()> { let request = ResponsesApiRequest { model: "gpt-test".into(), - instructions: "Say hi".into(), + instructions: Some("Say hi".into()), input: vec![ResponseItem::Message { id: Some("msg_1".into()), role: "user".into(), diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index d77e36870..b77dda2b7 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -399,7 +399,11 @@ impl ModelClient { ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth) .with_telemetry(Some(request_telemetry)); - let instructions = prompt.base_instructions.text.clone(); + let instructions = prompt + .base_instructions + .as_ref() + .map(|base_instructions| base_instructions.text.clone()) + .unwrap_or_default(); let input = prompt.get_formatted_input(); let tools = create_tools_json_for_responses_api(&prompt.tools)?; let reasoning = Self::build_reasoning(model_info, effort, summary); @@ -755,7 +759,10 @@ impl ModelClientSession { summary: ReasoningSummaryConfig, service_tier: Option, ) -> Result { - let instructions = &prompt.base_instructions.text; + let instructions = prompt + .base_instructions + .as_ref() + .map(|base_instructions| base_instructions.text.clone()); let input = prompt.get_formatted_input(); let tools = create_tools_json_for_responses_api(&prompt.tools)?; let default_reasoning_effort = model_info.default_reasoning_level; @@ -794,7 +801,7 @@ impl ModelClientSession { let prompt_cache_key = Some(self.client.state.conversation_id.to_string()); let request = ResponsesApiRequest { model: model_info.slug.clone(), - instructions: instructions.clone(), + instructions, input, tools, tool_choice: "auto".to_string(), diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index cb50ccac3..828839ae7 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -23,7 +23,7 @@ pub const REVIEW_EXIT_INTERRUPTED_TMPL: &str = include_str!("../templates/review/exit_interrupted.xml"); /// API request payload for a single model turn -#[derive(Default, Debug, Clone)] +#[derive(Debug, Clone)] pub struct Prompt { /// Conversation context input items. pub input: Vec, @@ -35,7 +35,7 @@ pub struct Prompt { /// Whether parallel tool calls are permitted for this prompt. pub(crate) parallel_tool_calls: bool, - pub base_instructions: BaseInstructions, + pub base_instructions: Option, /// Optionally specify the personality of the model. pub personality: Option, @@ -44,6 +44,19 @@ pub struct Prompt { pub output_schema: Option, } +impl Default for Prompt { + fn default() -> Self { + Self { + input: Vec::new(), + tools: Vec::new(), + parallel_tool_calls: false, + base_instructions: Some(BaseInstructions::default()), + personality: None, + output_schema: None, + } + } +} + impl Prompt { pub(crate) fn get_formatted_input(&self) -> Vec { let mut input = self.input.clone(); diff --git a/codex-rs/core/src/client_common_tests.rs b/codex-rs/core/src/client_common_tests.rs index 16a37796a..968703046 100644 --- a/codex-rs/core/src/client_common_tests.rs +++ b/codex-rs/core/src/client_common_tests.rs @@ -14,7 +14,7 @@ fn serializes_text_verbosity_when_set() { let tools: Vec = vec![]; let req = ResponsesApiRequest { model: "gpt-5.1".to_string(), - instructions: "i".to_string(), + instructions: Some("i".to_string()), input, tools, tool_choice: "auto".to_string(), @@ -57,7 +57,7 @@ fn serializes_text_schema_with_strict_format() { let req = ResponsesApiRequest { model: "gpt-5.1".to_string(), - instructions: "i".to_string(), + instructions: Some("i".to_string()), input, tools, tool_choice: "auto".to_string(), @@ -94,7 +94,7 @@ fn omits_text_when_not_set() { let tools: Vec = vec![]; let req = ResponsesApiRequest { model: "gpt-5.1".to_string(), - instructions: "i".to_string(), + instructions: Some("i".to_string()), input, tools, tool_choice: "auto".to_string(), @@ -116,7 +116,7 @@ fn omits_text_when_not_set() { fn serializes_flex_service_tier_when_set() { let req = ResponsesApiRequest { model: "gpt-5.1".to_string(), - instructions: "i".to_string(), + instructions: Some("i".to_string()), input: vec![], tools: vec![], tool_choice: "auto".to_string(), diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 423c62848..237b77e38 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -581,11 +581,15 @@ impl Codex { let model_info = models_manager .get_model_info(model.as_str(), &config.to_models_manager_config()) .await; - let 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)); + let base_instructions = match config.base_instructions.clone() { + Some(base_instructions) => base_instructions, + None => conversation_history + .get_base_instructions() + .map(|base_instructions| { + base_instructions.map(|base_instructions| base_instructions.text) + }) + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), + }; // Respect thread-start tools. When missing (resumed/forked threads), read from the db // first, then fall back to rollout-file tools. @@ -1106,7 +1110,7 @@ pub(crate) struct SessionConfiguration { personality: Option, /// Base instructions for the session. - base_instructions: String, + base_instructions: Option, /// Compact prompt override. compact_prompt: Option, @@ -1545,9 +1549,10 @@ impl Session { conversation_id, forked_from_id, session_source, - BaseInstructions { - text: session_configuration.base_instructions.clone(), - }, + session_configuration + .base_instructions + .clone() + .map(|text| BaseInstructions { text }), session_configuration.dynamic_tools.clone(), if session_configuration.persist_extended_history { EventPersistenceMode::Extended @@ -2109,8 +2114,9 @@ impl Session { )); } } - sess.schedule_startup_prewarm(session_configuration.base_instructions.clone()) - .await; + if let Some(base_instructions) = session_configuration.base_instructions.clone() { + sess.schedule_startup_prewarm(base_instructions).await; + } let session_start_source = match &initial_history { InitialHistory::Resumed(_) => codex_hooks::SessionStartSource::Resume, InitialHistory::New | InitialHistory::Forked(_) => { @@ -2212,11 +2218,13 @@ impl Session { state.history.estimate_token_count(turn_context) } - pub(crate) async fn get_base_instructions(&self) -> BaseInstructions { + pub(crate) async fn get_base_instructions(&self) -> Option { let state = self.state.lock().await; - BaseInstructions { - text: state.session_configuration.base_instructions.clone(), - } + state + .session_configuration + .base_instructions + .clone() + .map(|text| BaseInstructions { text }) } // Merges connector IDs into the session-level explicit connector selection. @@ -3620,7 +3628,11 @@ impl Session { state.reference_context_item(), state.previous_turn_settings(), state.session_configuration.collaboration_mode.clone(), - state.session_configuration.base_instructions.clone(), + state + .session_configuration + .base_instructions + .clone() + .unwrap_or_default(), state.session_configuration.session_source.clone(), ) }; @@ -3861,7 +3873,13 @@ impl Session { pub(crate) async fn recompute_token_usage(&self, turn_context: &TurnContext) { let history = self.clone_history().await; - let base_instructions = self.get_base_instructions().await; + let empty_base_instructions = BaseInstructions { + text: String::new(), + }; + let base_instructions = self + .get_base_instructions() + .await + .unwrap_or(empty_base_instructions); let Some(estimated_total_tokens) = history.estimate_token_count_with_base_instructions(&base_instructions) else { @@ -6555,7 +6573,7 @@ pub(crate) fn build_prompt( input: Vec, router: &ToolRouter, turn_context: &TurnContext, - base_instructions: BaseInstructions, + base_instructions: Option, ) -> Prompt { let deferred_dynamic_tools = turn_context .dynamic_tools diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index cf7c23be9..b0437edbf 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -591,11 +591,15 @@ async fn get_base_instructions_no_user_content() { { let mut state = session.state.lock().await; - state.session_configuration.base_instructions = model_info.base_instructions.clone(); + state.session_configuration.base_instructions = + Some(model_info.base_instructions.clone()); } let base_instructions = session.get_base_instructions().await; - assert_eq!(base_instructions.text, model_info.base_instructions); + assert_eq!( + base_instructions.expect("base instructions").text, + model_info.base_instructions + ); } } @@ -1091,7 +1095,7 @@ async fn recompute_token_usage_uses_session_base_instructions() { let override_instructions = "SESSION_OVERRIDE_INSTRUCTIONS_ONLY".repeat(120); { let mut state = session.state.lock().await; - state.session_configuration.base_instructions = override_instructions.clone(); + state.session_configuration.base_instructions = Some(override_instructions.clone()); } let item = user_message("hello"); @@ -1855,7 +1859,7 @@ async fn set_rate_limits_retains_previous_credits() { base_instructions: config .base_instructions .clone() - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, @@ -1957,7 +1961,7 @@ async fn set_rate_limits_updates_plan_type_when_present() { base_instructions: config .base_instructions .clone() - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, @@ -2222,7 +2226,7 @@ async fn attach_rollout_recorder(session: &Arc) -> PathBuf { ThreadId::default(), /*forked_from_id*/ None, SessionSource::Exec, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), EventPersistenceMode::Limited, ), @@ -2306,7 +2310,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati base_instructions: config .base_instructions .clone() - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, @@ -2572,7 +2576,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { base_instructions: config .base_instructions .clone() - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, @@ -2675,7 +2679,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { base_instructions: config .base_instructions .clone() - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, @@ -3515,7 +3519,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( base_instructions: config .base_instructions .clone() - .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + .unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))), compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, @@ -4264,7 +4268,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_baseline ThreadId::default(), /*forked_from_id*/ None, SessionSource::Exec, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), EventPersistenceMode::Limited, ), @@ -4361,7 +4365,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei ThreadId::default(), /*forked_from_id*/ None, SessionSource::Exec, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), EventPersistenceMode::Limited, ), diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 5bc7944d2..640ca3c1f 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -76,10 +76,16 @@ async fn run_remote_compact_task_inner_impl( .await; let mut history = sess.clone_history().await; let base_instructions = sess.get_base_instructions().await; + let token_count_base_instructions = + base_instructions + .clone() + .unwrap_or_else(|| BaseInstructions { + text: String::new(), + }); let deleted_items = trim_function_call_history_to_fit_context_window( &mut history, turn_context.as_ref(), - &base_instructions, + &token_count_base_instructions, ); if deleted_items > 0 { info!( @@ -127,8 +133,13 @@ async fn run_remote_compact_task_inner_impl( ) .or_else(|err| async { let total_usage_breakdown = sess.get_total_token_usage_breakdown().await; + let base_instruction_text = prompt + .base_instructions + .as_ref() + .map(|base_instructions| base_instructions.text.as_str()) + .unwrap_or(""); let compact_request_log_data = - build_compact_request_log_data(&prompt.input, &prompt.base_instructions.text); + build_compact_request_log_data(&prompt.input, base_instruction_text); log_remote_compact_failure( turn_context, &compact_request_log_data, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3c2d80464..20d3dd2f1 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -243,7 +243,7 @@ pub struct Config { pub user_instructions: Option, /// Base instructions override. - pub base_instructions: Option, + pub base_instructions: Option>, /// Developer instructions override injected as a separate message. pub developer_instructions: Option, @@ -687,7 +687,7 @@ impl Config { model_context_window: self.model_context_window, model_auto_compact_token_limit: self.model_auto_compact_token_limit, tool_output_token_limit: self.tool_output_token_limit, - base_instructions: self.base_instructions.clone(), + base_instructions: self.base_instructions.clone().flatten(), personality_enabled: self.features.enabled(Feature::Personality), model_supports_reasoning_summaries: self.model_supports_reasoning_summaries, model_catalog: self.model_catalog.clone(), @@ -1200,8 +1200,8 @@ pub struct ConfigOverrides { pub js_repl_node_path: Option, pub js_repl_node_module_dirs: Option>, pub zsh_path: Option, - pub base_instructions: Option, - pub developer_instructions: Option, + pub base_instructions: Option>, + pub developer_instructions: Option>, pub personality: Option, pub compact_prompt: Option, pub include_apply_patch_tool: Option, @@ -1760,8 +1760,9 @@ impl Config { .or(cfg.model_instructions_file.as_ref()); let file_base_instructions = Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?; - let base_instructions = base_instructions.or(file_base_instructions); - let developer_instructions = developer_instructions.or(cfg.developer_instructions); + let base_instructions = base_instructions.or_else(|| file_base_instructions.map(Some)); + let developer_instructions = + developer_instructions.unwrap_or_else(|| cfg.developer_instructions.clone()); let include_permissions_instructions = config_profile .include_permissions_instructions .or(cfg.include_permissions_instructions) diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 7978f37ec..629730cbf 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -905,7 +905,7 @@ model_instructions_file = "child.txt" .await?; assert_eq!( - config.base_instructions.as_deref(), + config.base_instructions.as_ref().and_then(Option::as_deref), Some("child instructions") ); @@ -941,7 +941,7 @@ async fn cli_override_model_instructions_file_sets_base_instructions() -> std::i .await?; assert_eq!( - config.base_instructions.as_deref(), + config.base_instructions.as_ref().and_then(Option::as_deref), Some("cli override instructions") ); diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index bc24974c8..368b812e5 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -137,7 +137,7 @@ impl GuardianReviewSessionReuseKey { model_reasoning_summary: spawn_config.model_reasoning_summary, permissions: spawn_config.permissions.clone(), developer_instructions: spawn_config.developer_instructions.clone(), - base_instructions: spawn_config.base_instructions.clone(), + base_instructions: spawn_config.base_instructions.clone().flatten(), user_instructions: spawn_config.user_instructions.clone(), compact_prompt: spawn_config.compact_prompt.clone(), cwd: spawn_config.cwd.to_path_buf(), diff --git a/codex-rs/core/src/memories/phase1.rs b/codex-rs/core/src/memories/phase1.rs index 37cb0e9f4..0b8cdcdad 100644 --- a/codex-rs/core/src/memories/phase1.rs +++ b/codex-rs/core/src/memories/phase1.rs @@ -336,9 +336,9 @@ mod job { }], tools: Vec::new(), parallel_tool_calls: false, - base_instructions: BaseInstructions { + base_instructions: Some(BaseInstructions { text: phase_one::PROMPT.to_string(), - }, + }), personality: None, output_schema: Some(output_schema()), }; diff --git a/codex-rs/core/src/session_startup_prewarm.rs b/codex-rs/core/src/session_startup_prewarm.rs index 397ffc949..708401aed 100644 --- a/codex-rs/core/src/session_startup_prewarm.rs +++ b/codex-rs/core/src/session_startup_prewarm.rs @@ -217,9 +217,9 @@ async fn schedule_startup_prewarm_inner( Vec::new(), startup_router.as_ref(), startup_turn_context.as_ref(), - BaseInstructions { + Some(BaseInstructions { text: base_instructions, - }, + }), ); let startup_turn_metadata_header = startup_turn_context .turn_metadata_state diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index a1cc07110..834cf20b4 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -112,7 +112,7 @@ async fn start_review_conversation( let _ = sub_agent_config.features.disable(Feature::Collab); // Set explicit review rubric for the sub-agent - sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string()); + sub_agent_config.base_instructions = Some(Some(crate::REVIEW_PROMPT.to_string())); sub_agent_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); let model = config diff --git a/codex-rs/core/src/tools/handlers/agent_jobs.rs b/codex-rs/core/src/tools/handlers/agent_jobs.rs index fe30f3d75..837b654ed 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs.rs @@ -535,7 +535,7 @@ async fn build_runner_options( let max_concurrency = normalize_concurrency(requested_concurrency, turn.config.agent_max_threads); let base_instructions = session.get_base_instructions().await; - let spawn_config = build_agent_spawn_config(&base_instructions, turn.as_ref())?; + let spawn_config = build_agent_spawn_config(base_instructions.as_ref(), turn.as_ref())?; Ok(JobRunnerOptions { max_concurrency, spawn_config, diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 8e4bfb5b5..a2482ea6a 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -59,8 +59,10 @@ impl ToolHandler for Handler { .into(), ) .await; - let mut config = - build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; + let mut config = build_agent_spawn_config( + session.get_base_instructions().await.as_ref(), + turn.as_ref(), + )?; apply_requested_spawn_agent_model_overrides( &session, turn.as_ref(), diff --git a/codex-rs/core/src/tools/handlers/multi_agents_common.rs b/codex-rs/core/src/tools/handlers/multi_agents_common.rs index 2078c229b..4205c4bb4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -201,11 +201,12 @@ pub(crate) fn parse_collab_input( /// skipping this helper and cloning stale config state directly can send the child agent out with /// the wrong provider or runtime policy. pub(crate) fn build_agent_spawn_config( - base_instructions: &BaseInstructions, + base_instructions: Option<&BaseInstructions>, turn: &TurnContext, ) -> Result { let mut config = build_agent_shared_config(turn)?; - config.base_instructions = Some(base_instructions.text.clone()); + config.base_instructions = + Some(base_instructions.map(|base_instructions| base_instructions.text.clone())); Ok(config) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index eab43f899..03f4b0780 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -3209,9 +3209,9 @@ async fn build_agent_spawn_config_uses_turn_context_values() { .set(AskForApproval::OnRequest) .expect("approval policy set"); - let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config"); + let config = build_agent_spawn_config(Some(&base_instructions), &turn).expect("spawn config"); let mut expected = (*turn.config).clone(); - expected.base_instructions = Some(base_instructions.text); + expected.base_instructions = Some(Some(base_instructions.text)); expected.model = Some(turn.model_info.slug.clone()); expected.model_provider = turn.provider.clone(); expected.model_reasoning_effort = turn.reasoning_effort; @@ -3247,7 +3247,7 @@ async fn build_agent_spawn_config_preserves_base_user_instructions() { text: "base".to_string(), }; - let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config"); + let config = build_agent_spawn_config(Some(&base_instructions), &turn).expect("spawn config"); assert_eq!(config.user_instructions, base_config.user_instructions); } @@ -3256,7 +3256,7 @@ async fn build_agent_spawn_config_preserves_base_user_instructions() { async fn build_agent_resume_config_clears_base_instructions() { let (_session, mut turn) = make_session_and_context().await; let mut base_config = (*turn.config).clone(); - base_config.base_instructions = Some("caller-base".to_string()); + base_config.base_instructions = Some(Some("caller-base".to_string())); turn.config = Arc::new(base_config); turn.approval_policy .set(AskForApproval::OnRequest) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 77c48af9e..a1cdb121d 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -68,8 +68,10 @@ impl ToolHandler for Handler { .into(), ) .await; - let mut config = - build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; + let mut config = build_agent_spawn_config( + session.get_base_instructions().await.as_ref(), + turn.as_ref(), + )?; apply_requested_spawn_agent_model_overrides( &session, turn.as_ref(), diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 8e719460e..cb97e4a3b 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -921,7 +921,7 @@ async fn includes_base_instructions_override_in_request() { let mut builder = test_codex() .with_auth(CodexAuth::from_api_key("Test API Key")) .with_config(|config| { - config.base_instructions = Some("test instructions".to_string()); + config.base_instructions = Some(Some("test instructions".to_string())); }); let codex = builder .build(&server) @@ -953,6 +953,47 @@ async fn includes_base_instructions_override_in_request() { ); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn omits_explicit_null_base_instructions_from_request() { + skip_if_no_network!(); + // Mock server + let server = MockServer::start().await; + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let mut builder = test_codex() + .with_auth(CodexAuth::from_api_key("Test API Key")) + .with_config(|config| { + config.base_instructions = Some(None); + }); + let codex = builder + .build(&server) + .await + .expect("create new conversation") + .codex; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + let request_body = request.body_json(); + + assert_eq!(request_body.get("instructions"), None); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn chatgpt_auth_sends_correct_request() { skip_if_no_network!(); diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index d348d4a02..0a1195a11 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -1654,9 +1654,9 @@ fn prompt_with_input(input: Vec) -> Prompt { fn prompt_with_input_and_instructions(input: Vec, instructions: &str) -> Prompt { let mut prompt = prompt_with_input(input); - prompt.base_instructions = BaseInstructions { + prompt.base_instructions = Some(BaseInstructions { text: instructions.to_string(), - }; + }); prompt } diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 0069c5fb8..dd99e0f39 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -869,7 +869,7 @@ async fn remote_compact_trim_estimate_uses_session_base_instructions() -> Result let override_base_instructions = override_base_instructions.clone(); move |config| { config.model_context_window = Some(override_context_window); - config.base_instructions = Some(override_base_instructions); + config.base_instructions = Some(Some(override_base_instructions)); } }), ) diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 172738ffd..8a0cbdcb6 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -66,7 +66,7 @@ async fn base_instructions_override_disables_personality_template() { .enable(Feature::Personality) .expect("test config should allow feature update"); config.personality = Some(Personality::Friendly); - config.base_instructions = Some("override instructions".to_string()); + config.base_instructions = Some(Some("override instructions".to_string())); let model_info = codex_core::test_support::construct_model_info_offline("gpt-5.2-codex", &config); diff --git a/codex-rs/core/tests/suite/rollout_list_find.rs b/codex-rs/core/tests/suite/rollout_list_find.rs index 7445a4ba0..93edd1621 100644 --- a/codex-rs/core/tests/suite/rollout_list_find.rs +++ b/codex-rs/core/tests/suite/rollout_list_find.rs @@ -173,7 +173,7 @@ async fn find_locates_rollout_file_written_by_recorder() -> std::io::Result<()> thread_id, /*forked_from_id*/ None, SessionSource::Exec, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), EventPersistenceMode::Limited, ), diff --git a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs index 19d9d27cf..c5aa57ce7 100644 --- a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs +++ b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs @@ -84,7 +84,7 @@ async fn continue_after_stream_error() { let TestCodex { codex, .. } = test_codex() .with_config(move |config| { - config.base_instructions = Some("You are a helpful assistant".to_string()); + config.base_instructions = Some(Some("You are a helpful assistant".to_string())); config.model_provider = provider; }) .build(&server) diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index f83fd4fd5..341f0ac4b 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -179,8 +179,8 @@ impl CodexToolCallParam { codex_self_exe: arg0_paths.codex_self_exe.clone(), codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(), - base_instructions, - developer_instructions, + base_instructions: base_instructions.map(Some), + developer_instructions: developer_instructions.map(Some), compact_prompt, ..Default::default() }; diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index de580de6f..e78bfe6e4 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -23,4 +23,5 @@ pub mod plan_tool; pub mod protocol; pub mod request_permissions; pub mod request_user_input; +mod serde_helpers; pub mod user_input; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 7116878b4..05926a954 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -48,6 +48,8 @@ use crate::plan_tool::UpdatePlanArgs; use crate::request_permissions::RequestPermissionsEvent; use crate::request_permissions::RequestPermissionsResponse; use crate::request_user_input::RequestUserInputResponse; +use crate::serde_helpers::deserialize_double_option; +use crate::serde_helpers::serialize_double_option; use crate::user_input::UserInput; use codex_git_utils::GitSha; use codex_utils_absolute_path::AbsolutePathBuf; @@ -2332,7 +2334,7 @@ impl InitialHistory { } } - pub fn get_base_instructions(&self) -> Option { + pub fn get_base_instructions(&self) -> Option> { // TODO: SessionMeta should (in theory) always be first in the history, so we can probably only check the first item? match self { InitialHistory::New => None, @@ -2540,7 +2542,13 @@ pub struct SessionMeta { /// base_instructions for the session. This *should* always be present when creating a new session, /// but may be missing for older sessions. If not present, fall back to rendering the base_instructions /// from ModelsManager. - pub base_instructions: Option, + #[serde( + default, + deserialize_with = "deserialize_double_option", + serialize_with = "serialize_double_option", + skip_serializing_if = "Option::is_none" + )] + pub base_instructions: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub dynamic_tools: Option>, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/codex-rs/protocol/src/serde_helpers.rs b/codex-rs/protocol/src/serde_helpers.rs new file mode 100644 index 000000000..0e35ebdba --- /dev/null +++ b/codex-rs/protocol/src/serde_helpers.rs @@ -0,0 +1,23 @@ +use serde::Deserialize; +use serde::Deserializer; +use serde::Serialize; +use serde::Serializer; + +pub fn deserialize_double_option<'de, T, D>(deserializer: D) -> Result>, D::Error> +where + T: Deserialize<'de>, + D: Deserializer<'de>, +{ + serde_with::rust::double_option::deserialize(deserializer) +} + +pub fn serialize_double_option( + value: &Option>, + serializer: S, +) -> Result +where + T: Serialize, + S: Serializer, +{ + serde_with::rust::double_option::serialize(value, serializer) +} diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index f39c38af6..aa7b485b0 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -81,7 +81,7 @@ pub enum RolloutRecorderParams { conversation_id: ThreadId, forked_from_id: Option, source: SessionSource, - base_instructions: BaseInstructions, + base_instructions: Option, dynamic_tools: Vec, event_persistence_mode: EventPersistenceMode, }, @@ -110,7 +110,7 @@ impl RolloutRecorderParams { conversation_id: ThreadId, forked_from_id: Option, source: SessionSource, - base_instructions: BaseInstructions, + base_instructions: Option, dynamic_tools: Vec, event_persistence_mode: EventPersistenceMode, ) -> Self { diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 163c8a1ee..6ee55672e 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -73,7 +73,7 @@ async fn recorder_materializes_only_after_explicit_persist() -> std::io::Result< thread_id, /*forked_from_id*/ None, SessionSource::Exec, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), EventPersistenceMode::Limited, ), @@ -166,7 +166,7 @@ async fn metadata_irrelevant_events_touch_state_db_updated_at() -> std::io::Resu thread_id, /*forked_from_id*/ None, SessionSource::Cli, - BaseInstructions::default(), + Some(BaseInstructions::default()), Vec::new(), EventPersistenceMode::Limited, ), diff --git a/codex-rs/rollout/src/tests.rs b/codex-rs/rollout/src/tests.rs index e647a54cd..aeda089a2 100644 --- a/codex-rs/rollout/src/tests.rs +++ b/codex-rs/rollout/src/tests.rs @@ -977,7 +977,7 @@ async fn test_get_thread_contents() { } #[tokio::test] -async fn test_base_instructions_missing_in_meta_defaults_to_null() { +async fn test_base_instructions_missing_in_meta_stays_missing() { let temp = TempDir::new().unwrap(); let home = temp.path(); @@ -1011,10 +1011,7 @@ async fn test_base_instructions_missing_in_meta_defaults_to_null() { .await .expect("session meta head"); let first = head.first().expect("first head entry"); - assert_eq!( - first.get("base_instructions"), - Some(&serde_json::Value::Null) - ); + assert_eq!(first.get("base_instructions"), None); } #[tokio::test]