mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Preserve developer instruction annotations in subagent forks (#40297)
## What changed - Add a dedicated contextual fragment for developer instructions that emits the `generic.developer_instructions` content kind. - Use the fragment when adding child-specific developer instructions to a full-history subagent fork, preserving their classification and ensuring they reach the child exactly once without appearing in the parent request. ## Testing - Extend full-history subagent coverage to verify the child-only instructions' annotation, placement, and count. GitOrigin-RevId: 82589d311535225fed76d500a19458fda72f40f2
This commit is contained in:
@@ -4,6 +4,7 @@ use crate::agent::role::apply_role_to_config;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context::CurrentTimeReminder;
|
||||
use crate::context::DeveloperInstructions;
|
||||
use crate::context::ManagedDeveloperInstructions;
|
||||
use crate::context::MultiAgentModeInstructions;
|
||||
use crate::context::MultiAgentRoleInstructions;
|
||||
@@ -845,11 +846,10 @@ impl AgentControl {
|
||||
.reference_context_item()
|
||||
.await
|
||||
.is_some()
|
||||
&& let Some(developer_message) =
|
||||
crate::context_manager::updates::build_developer_update_item(vec![
|
||||
subagent_developer_instructions.clone(),
|
||||
])
|
||||
{
|
||||
let developer_message = ContextualUserFragment::into(DeveloperInstructions::new(
|
||||
subagent_developer_instructions,
|
||||
));
|
||||
forked_rollout_items.push(RolloutItem::ResponseItem(developer_message.into()));
|
||||
}
|
||||
if preserve_reference_context_item
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
use super::ContextualUserFragment;
|
||||
use codex_protocol::models::ContentItemKind;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct DeveloperInstructions {
|
||||
instructions: String,
|
||||
}
|
||||
|
||||
impl DeveloperInstructions {
|
||||
pub(crate) fn new(instructions: impl Into<String>) -> Self {
|
||||
Self {
|
||||
instructions: instructions.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for DeveloperInstructions {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("generic.developer_instructions".to_string())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
"developer"
|
||||
}
|
||||
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
}
|
||||
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
("", "")
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
self.instructions.clone()
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,7 @@ mod base_instructions;
|
||||
mod compaction_summary;
|
||||
mod contextual_user_message;
|
||||
mod current_time_reminder;
|
||||
mod developer_instructions;
|
||||
mod environment_context;
|
||||
mod environments_instructions;
|
||||
mod guardian_approved_action;
|
||||
@@ -56,6 +57,7 @@ pub(crate) use compaction_summary::CompactionSummary;
|
||||
pub(crate) use contextual_user_message::is_contextual_user_fragment;
|
||||
pub(crate) use contextual_user_message::parse_visible_hook_prompt_message;
|
||||
pub(crate) use current_time_reminder::CurrentTimeReminder;
|
||||
pub(crate) use developer_instructions::DeveloperInstructions;
|
||||
pub(crate) use environments_instructions::EnvironmentsInstructions;
|
||||
pub(crate) use guardian_approved_action::GuardianApprovedAction;
|
||||
pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder;
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
use crate::context::ContextualUserFragment;
|
||||
use codex_context_fragments::AnnotatedContent;
|
||||
use codex_context_fragments::RenderedFragment;
|
||||
use codex_protocol::models::ContentItemKind;
|
||||
use codex_protocol::models::InternalChatMessageMetadataPassthrough;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
|
||||
@@ -11,20 +9,6 @@ enum MessageGroup {
|
||||
Mergeable,
|
||||
}
|
||||
|
||||
pub(crate) fn build_developer_update_item(text_sections: Vec<String>) -> Option<ResponseItem> {
|
||||
build_rendered_message(
|
||||
text_sections
|
||||
.into_iter()
|
||||
.map(|text| {
|
||||
RenderedFragment::new(
|
||||
"developer",
|
||||
AnnotatedContent::input_text(text, ContentItemKind("unknown".to_string())),
|
||||
)
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn build_rendered_message(fragments: Vec<RenderedFragment>) -> Option<ResponseItem> {
|
||||
let role = fragments.first()?.role();
|
||||
debug_assert!(fragments.iter().all(|fragment| fragment.role() == role));
|
||||
|
||||
@@ -21,6 +21,7 @@ use crate::compact::CompactedHistoryMetadata;
|
||||
use crate::config::ManagedFeatures;
|
||||
use crate::config::resolve_tool_suggest_config_from_layer_stack;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context::DeveloperInstructions;
|
||||
use crate::context::ManagedDeveloperInstructions;
|
||||
use crate::context::ModelSwitchInstructions;
|
||||
use crate::context::MultiAgentRoleInstructions;
|
||||
@@ -3544,13 +3545,8 @@ impl Session {
|
||||
&& let Some(developer_instructions) = turn_context.developer_instructions.as_deref()
|
||||
&& !developer_instructions.is_empty()
|
||||
{
|
||||
developer_sections.push(RenderedFragment::new(
|
||||
"developer",
|
||||
AnnotatedContent::input_text(
|
||||
developer_instructions,
|
||||
ContentItemKind("generic.developer_instructions".to_string()),
|
||||
),
|
||||
));
|
||||
developer_sections
|
||||
.push(DeveloperInstructions::new(developer_instructions).render_fragment());
|
||||
}
|
||||
let loaded_plugins = self
|
||||
.services
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::config::ConfigBuilder;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::test_config;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context::DeveloperInstructions;
|
||||
use crate::context::TurnAborted;
|
||||
use crate::environment_selection::EnvironmentConfigOrigin;
|
||||
use crate::environment_selection::ThreadEnvironments;
|
||||
@@ -3335,10 +3336,8 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item
|
||||
.await;
|
||||
let rollout_path =
|
||||
attach_thread_persistence(Arc::get_mut(&mut session).expect("unique session")).await;
|
||||
let response_item = crate::context_manager::updates::build_developer_update_item(vec![
|
||||
"Subagent guidance.".to_string(),
|
||||
])
|
||||
.expect("developer message");
|
||||
let response_item =
|
||||
ContextualUserFragment::into(DeveloperInstructions::new("Subagent guidance."));
|
||||
let mut expected_item = ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
@@ -3347,7 +3346,9 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: Some(InternalChatMessageMetadataPassthrough {
|
||||
content_item_kinds: Some(vec![ContentItemKind("unknown".to_string())]),
|
||||
content_item_kinds: Some(vec![ContentItemKind(
|
||||
"generic.developer_instructions".to_string(),
|
||||
)]),
|
||||
..Default::default()
|
||||
}),
|
||||
};
|
||||
|
||||
@@ -84,6 +84,8 @@ const SUBAGENT_START_CONTEXT: &str = "subagent start context reaches child";
|
||||
const SUBAGENT_STOP_CONTINUATION: &str = "continue only the child";
|
||||
const INTERNAL_SUBAGENT_PROMPT: &str = "internal subagent: review";
|
||||
const FULL_HISTORY_MULTI_AGENT_MODE_HINT: &str = "Delegate independent work to another agent.";
|
||||
const FULL_HISTORY_SUBAGENT_DEVELOPER_INSTRUCTIONS: &str =
|
||||
"Child-only developer instructions preserve their classification.";
|
||||
const FULL_HISTORY_SHARED_USAGE_HINT: &str = "Shared delegation guidance.";
|
||||
const FULL_HISTORY_PROACTIVE_PROMPT: &str = "switch to proactive delegation";
|
||||
const FULL_HISTORY_EXPLICIT_PROMPT: &str = "restore explicit-only delegation";
|
||||
@@ -1229,6 +1231,11 @@ async fn spawned_full_history_v2_child_uses_model_precedence_without_dropping_co
|
||||
.expect("test config should allow feature update");
|
||||
config.model_context_window = Some(128_000);
|
||||
}
|
||||
if matches!(selection, FullHistoryV2ModelSelection::ConfiguredDefault) {
|
||||
config.developer_instructions = None;
|
||||
config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some(FULL_HISTORY_SUBAGENT_DEVELOPER_INSTRUCTIONS.to_string());
|
||||
}
|
||||
if matches!(selection, FullHistoryV2ModelSelection::CurrentTimeReminders) {
|
||||
config
|
||||
.features
|
||||
@@ -1341,6 +1348,19 @@ async fn spawned_full_history_v2_child_uses_model_precedence_without_dropping_co
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(misaligned_child_messages, Vec::<Value>::new());
|
||||
let child_developer_messages = child_request.message_input_texts("developer");
|
||||
if matches!(selection, FullHistoryV2ModelSelection::ConfiguredDefault) {
|
||||
assert_eq!(
|
||||
(
|
||||
parent_request.body_contains_text(FULL_HISTORY_SUBAGENT_DEVELOPER_INSTRUCTIONS),
|
||||
child_request.has_content_kinds(&["generic.developer_instructions"]),
|
||||
child_developer_messages
|
||||
.iter()
|
||||
.filter(|text| text.as_str() == FULL_HISTORY_SUBAGENT_DEVELOPER_INSTRUCTIONS)
|
||||
.count(),
|
||||
),
|
||||
(false, true, 1)
|
||||
);
|
||||
}
|
||||
if !matches!(
|
||||
selection,
|
||||
FullHistoryV2ModelSelection::MultiAgentModeTransitions
|
||||
|
||||
Reference in New Issue
Block a user