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 a7ba311803..952da565a6 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 770344de8e..fcde0ae502 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 61f501ad60..c7b27823f6 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 61b7db45e2..3562d9cda6 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 920fcbeceb..6c3b4e6562 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 { @@ -2489,8 +2489,8 @@ impl CodexMessageProcessor { sandbox_mode: sandbox.map(SandboxMode::to_core), codex_linux_sandbox_exe: self.arg0_paths.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: self.arg0_paths.main_execve_wrapper_exe.clone(), - base_instructions, - developer_instructions, + base_instructions: thread_instruction_override(base_instructions), + developer_instructions: thread_instruction_override(developer_instructions), personality, ..Default::default() } @@ -7843,6 +7843,14 @@ fn normalize_thread_list_cwd_filter( }) } +fn thread_instruction_override(instructions: Option>) -> Option { + match instructions { + None => None, + Some(None) => Some(String::new()), + Some(Some(instructions)) => Some(instructions), + } +} + #[cfg(test)] mod thread_list_cwd_filter_tests { use super::normalize_thread_list_cwd_filter; diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 80ae756888..a8277e04f9 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -1772,6 +1772,7 @@ async fn thread_resume_accepts_personality_override() -> Result<()> { let start_id = primary .send_thread_start_request(ThreadStartParams { model: Some("gpt-5.2-codex".to_string()), + base_instructions: Some(Some(CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT.to_string())), ..Default::default() }) .await?; @@ -1810,6 +1811,7 @@ async fn thread_resume_accepts_personality_override() -> Result<()> { .send_thread_resume_request(ThreadResumeParams { thread_id: thread.id, model: Some("gpt-5.2-codex".to_string()), + base_instructions: Some(Some(CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT.to_string())), personality: Some(Personality::Friendly), ..Default::default() }) 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 59591dd0d1..8852dcf716 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, + "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.is_empty(), + "did not expect 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")?]; @@ -944,10 +1041,12 @@ async fn turn_start_uses_migrated_pragmatic_personality_without_override_v2() -> .await??; let request = response_mock.single_request(); - let instructions_text = request.instructions_text(); + let developer_texts = request.message_input_texts("developer"); assert!( - instructions_text.contains(LOCAL_PRAGMATIC_TEMPLATE), - "expected startup-migrated pragmatic personality in model instructions, got: {instructions_text:?}" + developer_texts + .iter() + .any(|text| text.contains(LOCAL_PRAGMATIC_TEMPLATE)), + "expected startup-migrated pragmatic personality in developer instructions, got: {developer_texts:?}" ); Ok(()) diff --git a/codex-rs/codex-api/src/common.rs b/codex-rs/codex-api/src/common.rs index 39fb976e67..a2a2c1d478 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 ba0f416788..2f3bc17e2c 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 d77e36870a..ef184aa376 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -755,7 +755,11 @@ impl ModelClientSession { summary: ReasoningSummaryConfig, service_tier: Option, ) -> Result { - let instructions = &prompt.base_instructions.text; + let instructions = if prompt.base_instructions.text.is_empty() { + None + } else { + Some(prompt.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 +798,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_tests.rs b/codex-rs/core/src/client_common_tests.rs index 16a37796ad..968703046e 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 4bf309c902..aa86a4b7db 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -3652,6 +3652,7 @@ impl Session { // stays isolated as its own top-level developer message for guardian subagents. if !separate_guardian_developer_message && let Some(developer_instructions) = turn_context.developer_instructions.as_deref() + && !developer_instructions.is_empty() { developer_sections.push(developer_instructions.to_string()); } @@ -3766,6 +3767,7 @@ impl Session { // subagent sees a distinct, easy-to-audit instruction block. if separate_guardian_developer_message && let Some(developer_instructions) = turn_context.developer_instructions.as_deref() + && !developer_instructions.is_empty() && let Some(guardian_developer_message) = crate::context_manager::updates::build_developer_update_item(vec![ developer_instructions.to_string(),