codex: address null instruction review feedback (#16964)

codex-rs/core/src/codex.rs

@@ -3871,12 +3871,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
-                .unwrap_or_else(|| BaseInstructions {
-                    text: String::new(),
-                });
+        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 {

codex-rs/core/tests/suite/client.rs

@@ -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!();

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;

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;
@@ -2542,8 +2544,8 @@ pub struct SessionMeta {
     /// from ModelsManager.
     #[serde(
         default,
-        deserialize_with = "serde_with::rust::double_option::deserialize",
-        serialize_with = "serde_with::rust::double_option::serialize",
+        deserialize_with = "deserialize_double_option",
+        serialize_with = "serialize_double_option",
         skip_serializing_if = "Option::is_none"
     )]
     pub base_instructions: Option<Option<BaseInstructions>>,

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<Option<Option<T>>, D::Error>
+where
+    T: Deserialize<'de>,
+    D: Deserializer<'de>,
+{
+    serde_with::rust::double_option::deserialize(deserializer)
+}
+
+pub fn serialize_double_option<T, S>(
+    value: &Option<Option<T>>,
+    serializer: S,
+) -> Result<S::Ok, S::Error>
+where
+    T: Serialize,
+    S: Serializer,
+{
+    serde_with::rust::double_option::serialize(value, serializer)
+}