mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Omit empty app-server instruction overrides (#17258)
## Summary - omit serialized Responses instructions when an app-server base instruction override is empty - skip empty developer instruction messages and add v2 coverage for the empty-override request shape ## Validation - just fmt - git diff --check
This commit is contained in:
committed by
GitHub
Unverified
parent
ff1ab61e4f
commit
ecca34209d
@@ -238,6 +238,96 @@ async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_omits_empty_instruction_overrides_from_model_request() -> Result<()> {
|
||||
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_once(&server, body).await;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(
|
||||
codex_home.path(),
|
||||
&server.uri(),
|
||||
"never",
|
||||
&BTreeMap::default(),
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
// TODO(aibrahim): Replace empty string instruction overrides with explicit tri-state
|
||||
// app-server semantics: omitted, explicitly none, or explicit value.
|
||||
config: Some(HashMap::from([(
|
||||
"include_permissions_instructions".to_string(),
|
||||
json!(false),
|
||||
)])),
|
||||
base_instructions: Some(String::new()),
|
||||
developer_instructions: Some(String::new()),
|
||||
..Default::default()
|
||||
})
|
||||
.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 request_body = response_mock.single_request().body_json();
|
||||
let empty_developer_input_texts = request_body["input"]
|
||||
.as_array()
|
||||
.expect("input array")
|
||||
.iter()
|
||||
.filter(|item| item.get("role").and_then(serde_json::Value::as_str) == Some("developer"))
|
||||
.filter_map(|item| item.get("content").and_then(serde_json::Value::as_array))
|
||||
.flatten()
|
||||
.filter(|content| {
|
||||
content.get("type").and_then(serde_json::Value::as_str) == Some("input_text")
|
||||
})
|
||||
.filter_map(|content| content.get("text").and_then(serde_json::Value::as_str))
|
||||
.filter(|text| text.is_empty())
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
json!({
|
||||
"hasInstructions": request_body.get("instructions").is_some(),
|
||||
"emptyDeveloperInputTexts": empty_developer_input_texts,
|
||||
}),
|
||||
json!({
|
||||
"hasInstructions": false,
|
||||
"emptyDeveloperInputTexts": [],
|
||||
})
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_accepts_text_at_limit_with_mention_item() -> Result<()> {
|
||||
let responses = vec![create_final_assistant_message_sse_response("Done")?];
|
||||
|
||||
@@ -24,6 +24,7 @@ pub const WS_REQUEST_HEADER_TRACESTATE_CLIENT_METADATA_KEY: &str = "ws_request_h
|
||||
pub struct CompactionInput<'a> {
|
||||
pub model: &'a str,
|
||||
pub input: &'a [ResponseItem],
|
||||
#[serde(skip_serializing_if = "str::is_empty")]
|
||||
pub instructions: &'a str,
|
||||
pub tools: Vec<Value>,
|
||||
pub parallel_tool_calls: bool,
|
||||
@@ -153,6 +154,7 @@ impl From<VerbosityConfig> for OpenAiVerbosity {
|
||||
#[derive(Debug, Serialize, Clone, PartialEq)]
|
||||
pub struct ResponsesApiRequest {
|
||||
pub model: String,
|
||||
#[serde(skip_serializing_if = "String::is_empty")]
|
||||
pub instructions: String,
|
||||
pub input: Vec<ResponseItem>,
|
||||
pub tools: Vec<serde_json::Value>,
|
||||
@@ -198,6 +200,7 @@ impl From<&ResponsesApiRequest> for ResponseCreateWsRequest {
|
||||
#[derive(Debug, Serialize)]
|
||||
pub struct ResponseCreateWsRequest {
|
||||
pub model: String,
|
||||
#[serde(skip_serializing_if = "String::is_empty")]
|
||||
pub instructions: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub previous_response_id: Option<String>,
|
||||
|
||||
@@ -3746,6 +3746,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());
|
||||
}
|
||||
@@ -3860,6 +3861,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(),
|
||||
|
||||
Reference in New Issue
Block a user