openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-05 20:10:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Honor null thread instructions

Browse Source
This commit is contained in:
Ahmed Ibrahim
2026-04-06 19:44:51 -07:00
parent 9d13d29acd
commit 11b179bdee
12 changed files with 208 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts
+1 -1
View File
@@ -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.
*/
codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts
+1 -1
View File
@@ -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.
*/
codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts
+1 -1
View File
@@ -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).
*/
codex-rs/app-server-protocol/src/protocol/v2.rs
+71 -6
View File
@@ -2603,10 +2603,22 @@ pub struct ThreadStartParams {
pub config: Option<HashMap<String, JsonValue>>,
#[ts(optional = nullable)]
pub service_name: Option<String>,
#[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<String>,
pub base_instructions: Option<Option<String>>,
#[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<String>,
pub developer_instructions: Option<Option<String>>,
#[ts(optional = nullable)]
pub personality: Option<Personality>,
#[ts(optional = nullable)]
@@ -2721,10 +2733,22 @@ pub struct ThreadResumeParams {
pub sandbox: Option<SandboxMode>,
#[ts(optional = nullable)]
pub config: Option<HashMap<String, serde_json::Value>>,
#[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<String>,
pub base_instructions: Option<Option<String>>,
#[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<String>,
pub developer_instructions: Option<Option<String>>,
#[ts(optional = nullable)]
pub personality: Option<Personality>,
/// If true, persist additional rollout EventMsg variants required to
@@ -2798,10 +2822,22 @@ pub struct ThreadForkParams {
pub sandbox: Option<SandboxMode>,
#[ts(optional = nullable)]
pub config: Option<HashMap<String, serde_json::Value>>,
#[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<String>,
pub base_instructions: Option<Option<String>>,
#[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<String>,
pub developer_instructions: Option<Option<String>>,
#[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(&params).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!({
codex-rs/app-server/src/codex_message_processor.rs
+12 -4
View File
@@ -2473,8 +2473,8 @@ impl CodexMessageProcessor {
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
sandbox: Option<SandboxMode>,
base_instructions: Option<String>,
developer_instructions: Option<String>,
base_instructions: Option<Option<String>>,
developer_instructions: Option<Option<String>>,
personality: Option<Personality>,
) -> 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<String>>) -> Option<String> {
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;
codex-rs/app-server/tests/suite/v2/thread_resume.rs
+2
View File
@@ -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()
})
codex-rs/app-server/tests/suite/v2/turn_start.rs
+102 -3
View File
@@ -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::<ThreadStartResponse>(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(())
codex-rs/codex-api/src/common.rs
+4 -2
View File
@@ -153,7 +153,8 @@ impl From<VerbosityConfig> 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<String>,
pub input: Vec<ResponseItem>,
pub tools: Vec<serde_json::Value>,
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<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub previous_response_id: Option<String>,
pub input: Vec<ResponseItem>,
codex-rs/codex-api/tests/clients.rs
+2 -2
View File
@@ -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(),
codex-rs/core/src/client.rs
+6 -2
View File
@@ -755,7 +755,11 @@ impl ModelClientSession {
summary: ReasoningSummaryConfig,
service_tier: Option<ServiceTier>,
) -> Result<ResponsesApiRequest> {
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(),
codex-rs/core/src/client_common_tests.rs
+4 -4
View File
@@ -14,7 +14,7 @@ fn serializes_text_verbosity_when_set() {
let tools: Vec<serde_json::Value> = 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<serde_json::Value> = 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(),
codex-rs/core/src/codex.rs
+2
View File
@@ -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(),