mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Preserve content annotations when filtering forked agent history (#40266)
## Why Developer-message content and its positional `content_item_kinds` metadata must stay aligned when parent history is prepared for a spawned agent. ## What changed - Filter and rewrite developer-message content as annotated content so each retained item keeps its matching kind. - Rebuild both message content and annotation metadata together after removing parent-only instructions and multi-agent usage hints. ## Testing - Extend forked-history coverage to verify retained annotation kinds. - Assert that spawned child requests contain the same number of content items and annotation kinds. GitOrigin-RevId: 59748d9e8c4fc06ef8e5b90389bcf40465589f90
This commit is contained in:
@@ -8,6 +8,8 @@ use crate::context::ManagedDeveloperInstructions;
|
||||
use crate::context::MultiAgentModeInstructions;
|
||||
use crate::context::MultiAgentRoleInstructions;
|
||||
use crate::session::multi_agents::resolve_usage_hints;
|
||||
use codex_context_fragments::set_annotated_content;
|
||||
use codex_context_fragments::to_annotated_content;
|
||||
use codex_extension_api::ExtensionDataInit;
|
||||
|
||||
const AGENT_NAMES: &str = include_str!("../agent_names.txt");
|
||||
@@ -91,15 +93,15 @@ fn keep_forked_rollout_item(item: &RolloutItem, preserve_reference_context_item:
|
||||
}
|
||||
|
||||
fn retain_forked_developer_message(item: &mut ResponseItem, usage_hint_texts: &[String]) -> bool {
|
||||
let ResponseItem::Message { role, content, .. } = item else {
|
||||
return true;
|
||||
};
|
||||
if role != "developer" {
|
||||
if !matches!(item, ResponseItem::Message { role, .. } if role == "developer") {
|
||||
return true;
|
||||
}
|
||||
|
||||
let Some(mut content) = to_annotated_content(item) else {
|
||||
return false;
|
||||
};
|
||||
content.retain(|content_item| {
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
let ContentItem::InputText { text } = content_item.content() else {
|
||||
return true;
|
||||
};
|
||||
|
||||
@@ -110,7 +112,7 @@ fn retain_forked_developer_message(item: &mut ResponseItem, usage_hint_texts: &[
|
||||
.iter()
|
||||
.any(|usage_hint_text| usage_hint_text == text))
|
||||
});
|
||||
!content.is_empty()
|
||||
!content.is_empty() && set_annotated_content(item, content).is_some()
|
||||
}
|
||||
|
||||
async fn load_agent_model_context(
|
||||
@@ -741,11 +743,12 @@ impl AgentControl {
|
||||
return false;
|
||||
}
|
||||
|
||||
if let ResponseItem::Message { role, content, .. } = response_item
|
||||
&& role == "developer"
|
||||
{
|
||||
if matches!(response_item, ResponseItem::Message { role, .. } if role == "developer") {
|
||||
let Some(mut content) = to_annotated_content(response_item) else {
|
||||
return false;
|
||||
};
|
||||
content.retain_mut(|content_item| {
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
let ContentItem::InputText { text } = content_item.content_mut() else {
|
||||
return true;
|
||||
};
|
||||
if ManagedDeveloperInstructions::matches_text(text) {
|
||||
@@ -777,7 +780,8 @@ impl AgentControl {
|
||||
*text = text.replace(parent_developer_instructions, replacement);
|
||||
!text.is_empty()
|
||||
});
|
||||
return !content.is_empty();
|
||||
return !content.is_empty()
|
||||
&& set_annotated_content(response_item, content).is_some();
|
||||
}
|
||||
|
||||
true
|
||||
|
||||
@@ -35,7 +35,9 @@ use codex_protocol::error::CodexErrorDetails;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::UserMessageItem;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ContentItemKind;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::InternalChatMessageMetadataPassthrough;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
@@ -1404,7 +1406,17 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
},
|
||||
],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
internal_chat_message_metadata_passthrough: Some(
|
||||
InternalChatMessageMetadataPassthrough {
|
||||
content_item_kinds: Some(vec![
|
||||
ContentItemKind("generic.developer_instructions".to_string()),
|
||||
ContentItemKind("multi_agent.mode_instructions".to_string()),
|
||||
ContentItemKind("generic.developer_policy".to_string()),
|
||||
ContentItemKind("managed_config.developer_instructions".to_string()),
|
||||
]),
|
||||
..Default::default()
|
||||
},
|
||||
),
|
||||
},
|
||||
assistant_message("parent commentary", Some(MessagePhase::Commentary)),
|
||||
assistant_message("parent final answer", Some(MessagePhase::FinalAnswer)),
|
||||
@@ -1499,7 +1511,16 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
},
|
||||
],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
internal_chat_message_metadata_passthrough: Some(
|
||||
InternalChatMessageMetadataPassthrough {
|
||||
content_item_kinds: Some(vec![
|
||||
ContentItemKind("generic.developer_instructions".to_string()),
|
||||
ContentItemKind("generic.developer_policy".to_string()),
|
||||
ContentItemKind("managed_config.developer_instructions".to_string()),
|
||||
]),
|
||||
..Default::default()
|
||||
},
|
||||
),
|
||||
};
|
||||
expected_developer_message.set_turn_id_if_missing(&turn_context.sub_id);
|
||||
expected_developer_message.set_create_time_if_missing(
|
||||
|
||||
@@ -1328,6 +1328,20 @@ async fn spawned_full_history_v2_child_uses_model_precedence_without_dropping_co
|
||||
|
||||
let child_request = wait_for_request_with_model(&child_request_log, expected_model).await?;
|
||||
assert!(child_request.body_contains_text(TURN_0_FORK_PROMPT));
|
||||
let misaligned_child_messages = child_request
|
||||
.inputs_of_type("message")
|
||||
.into_iter()
|
||||
.filter(|message| {
|
||||
message["internal_chat_message_metadata_passthrough"]["content_item_kinds"]
|
||||
.as_array()
|
||||
.is_some_and(|kinds| {
|
||||
message["content"]
|
||||
.as_array()
|
||||
.is_none_or(|content| content.len() != kinds.len())
|
||||
})
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(misaligned_child_messages, Vec::<Value>::new());
|
||||
let child_developer_messages = child_request.message_input_texts("developer");
|
||||
if !matches!(
|
||||
selection,
|
||||
|
||||
Reference in New Issue
Block a user