mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Add configurable developer instructions for v2 subagents (#35708)
## What changed - Add `features.multi_agent_v2.subagent_developer_instructions` to override inherited developer instructions for subagents that do not define role-specific instructions. - Preserve role-specific instructions as the higher-precedence setting, and carry the effective instructions through full and bounded forks, compacted histories, and cold resume without duplication. - Treat an unset override as inheritance and a blank override as clearing inherited instructions. ## Testing - Cover configuration parsing and materialization, instruction precedence, fork modes, compaction, role application, and resumed subagents. GitOrigin-RevId: 32321e61abc59b14a1be5f31e6bd6570bb1cd89d
This commit is contained in:
committed by
copyberry
parent
3418498f01
commit
49025589b0
Generated
+1
@@ -2030,6 +2030,7 @@ dependencies = [
|
||||
"shlex",
|
||||
"tar",
|
||||
"tempfile",
|
||||
"test-case",
|
||||
"thiserror 2.0.18",
|
||||
"time",
|
||||
"tokio",
|
||||
|
||||
@@ -138,6 +138,7 @@ serial_test = { workspace = true }
|
||||
sha2 = { workspace = true }
|
||||
shlex = { workspace = true }
|
||||
tar = { workspace = true }
|
||||
test-case = "3.3.1"
|
||||
tokio-tungstenite = { workspace = true }
|
||||
tracing-opentelemetry = { workspace = true }
|
||||
wiremock = { workspace = true }
|
||||
|
||||
@@ -11,21 +11,436 @@ use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use core_test_support::responses;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::time::Duration;
|
||||
use tempfile::TempDir;
|
||||
use test_case::test_case;
|
||||
use tokio::time::timeout;
|
||||
|
||||
#[cfg(windows)]
|
||||
const READ_TIMEOUT: Duration = Duration::from_secs(25);
|
||||
#[cfg(not(windows))]
|
||||
const READ_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
const NAMESPACE: &str = "collaboration";
|
||||
const PARENT_INSTRUCTIONS: &str = "parent-only developer instructions";
|
||||
const CHILD_INSTRUCTIONS: &str = "child-only developer instructions";
|
||||
const ROLE_INSTRUCTIONS: &str = "configured role developer instructions";
|
||||
|
||||
/// A roleless worker keeps inherited developer instructions across cold root resume and lazy reload.
|
||||
/// V2 fork modes, roles, and unset/blank overrides expose their agreed instruction precedence.
|
||||
#[test_case("no history"; "no history")]
|
||||
#[test_case("full history"; "full history")]
|
||||
#[test_case("bounded history"; "bounded history")]
|
||||
#[test_case("configured role without instructions"; "configured role without instructions")]
|
||||
#[test_case("unset override"; "unset override")]
|
||||
#[test_case("blank override"; "blank override")]
|
||||
#[test_case("parent has no instructions"; "parent has no instructions")]
|
||||
#[test_case("explicit configured role"; "explicit configured role")]
|
||||
#[test_case("implicit configured default"; "implicit configured default")]
|
||||
#[test_case("full fork skips default role"; "full fork skips default role")]
|
||||
#[tokio::test]
|
||||
async fn cold_resume_preserves_inherited_developer_instructions_for_roleless_worker() -> Result<()>
|
||||
{
|
||||
const PARENT_INSTRUCTIONS: &str = "parent-only developer instructions";
|
||||
async fn spawned_subagents_apply_configured_developer_instruction_precedence(
|
||||
case: &str,
|
||||
) -> Result<()> {
|
||||
let fork_turns = match case {
|
||||
"bounded history" => Some("1"),
|
||||
"no history"
|
||||
| "configured role without instructions"
|
||||
| "explicit configured role"
|
||||
| "implicit configured default" => Some("none"),
|
||||
_ => None,
|
||||
};
|
||||
let agent_type = match case {
|
||||
"configured role without instructions" | "explicit configured role" => Some("custom"),
|
||||
_ => None,
|
||||
};
|
||||
let configured_override = match case {
|
||||
"unset override" => None,
|
||||
"blank override" => Some(" "),
|
||||
"full history" => Some(" child-only developer instructions "),
|
||||
_ => Some(CHILD_INSTRUCTIONS),
|
||||
};
|
||||
let parent = if case == "parent has no instructions" {
|
||||
None
|
||||
} else {
|
||||
Some(PARENT_INSTRUCTIONS)
|
||||
};
|
||||
let configured_roles = matches!(
|
||||
case,
|
||||
"configured role without instructions"
|
||||
| "explicit configured role"
|
||||
| "implicit configured default"
|
||||
| "full fork skips default role"
|
||||
);
|
||||
let role_has_instructions = matches!(
|
||||
case,
|
||||
"explicit configured role" | "implicit configured default" | "full fork skips default role"
|
||||
);
|
||||
let expected = match case {
|
||||
"unset override" => Some(PARENT_INSTRUCTIONS),
|
||||
"blank override" => None,
|
||||
"explicit configured role" | "implicit configured default" => Some(ROLE_INSTRUCTIONS),
|
||||
_ => Some(CHILD_INSTRUCTIONS),
|
||||
};
|
||||
const PARENT_PROMPT: &str = "spawn the instruction override worker";
|
||||
const CHILD_PROMPT: &str = "perform the instruction override task";
|
||||
const SPAWN_CALL_ID: &str = "spawn-instruction-override-worker";
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mut spawn_args = json!({"message": CHILD_PROMPT, "task_name": "worker"});
|
||||
if let Some(fork_turns) = fork_turns {
|
||||
spawn_args["fork_turns"] = json!(fork_turns);
|
||||
}
|
||||
if let Some(agent_type) = agent_type {
|
||||
spawn_args["agent_type"] = json!(agent_type);
|
||||
}
|
||||
let parent_request = responses::mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| {
|
||||
String::from_utf8_lossy(&request.body).contains(PARENT_PROMPT)
|
||||
},
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("parent-spawn"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
SPAWN_CALL_ID,
|
||||
NAMESPACE,
|
||||
"spawn_agent",
|
||||
&serde_json::to_string(&spawn_args)?,
|
||||
),
|
||||
responses::ev_completed("parent-spawn"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let child_request = responses::mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| {
|
||||
let body = String::from_utf8_lossy(&request.body);
|
||||
body.contains(CHILD_PROMPT) && !body.contains(SPAWN_CALL_ID)
|
||||
},
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("child-work"),
|
||||
responses::ev_assistant_message("child-message", "child complete"),
|
||||
responses::ev_completed("child-work"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
responses::mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| {
|
||||
String::from_utf8_lossy(&request.body).contains(SPAWN_CALL_ID)
|
||||
},
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("parent-complete"),
|
||||
responses::ev_assistant_message("parent-message", "parent complete"),
|
||||
responses::ev_completed("parent-complete"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut feature_config = "[features.multi_agent_v2]\nenabled = true".to_string();
|
||||
if let Some(configured_override) = configured_override {
|
||||
feature_config.push_str(&format!(
|
||||
"\nsubagent_developer_instructions = {configured_override:?}"
|
||||
));
|
||||
}
|
||||
if configured_roles {
|
||||
feature_config.push_str(
|
||||
"\n\n[agents.custom]\ndescription = \"configured role\"\nconfig_file = \"./config.toml\"\n\n[agents.default]\ndescription = \"configured default role\"\nconfig_file = \"./config.toml\"",
|
||||
);
|
||||
}
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut config = MockResponsesConfig::new(&server.uri()).with_model("gpt-5.4");
|
||||
if role_has_instructions {
|
||||
config =
|
||||
config.with_root_config(&format!("developer_instructions = {ROLE_INSTRUCTIONS:?}"));
|
||||
}
|
||||
config
|
||||
.with_extra_config(&feature_config)
|
||||
.write(codex_home.path())?;
|
||||
write_models_cache(codex_home.path())?;
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } = app_server
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
developer_instructions: parent.map(str::to_string),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let _: TurnStartResponse = app_server
|
||||
.request(|request_id| ClientRequest::TurnStart {
|
||||
request_id,
|
||||
params: TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: PARENT_PROMPT.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let child_request = timeout(READ_TIMEOUT, async {
|
||||
loop {
|
||||
if let Some(request) = child_request
|
||||
.requests()
|
||||
.into_iter()
|
||||
.find(|request| !request.inputs_of_type("agent_message").is_empty())
|
||||
{
|
||||
break request;
|
||||
}
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
|
||||
let parent_texts = parent_request
|
||||
.single_request()
|
||||
.message_input_texts("developer");
|
||||
if let Some(parent) = parent {
|
||||
assert!(
|
||||
parent_texts.iter().any(|text| text == parent),
|
||||
"{case}: parent developer instructions unexpectedly changed: {parent_texts:?}"
|
||||
);
|
||||
}
|
||||
let child_texts = child_request.message_input_texts("developer");
|
||||
let instruction_texts = child_texts
|
||||
.iter()
|
||||
.map(String::as_str)
|
||||
.filter(|text| {
|
||||
matches!(
|
||||
*text,
|
||||
PARENT_INSTRUCTIONS | CHILD_INSTRUCTIONS | ROLE_INSTRUCTIONS
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let expected_instruction_texts = match expected {
|
||||
Some(instructions) => vec![instructions],
|
||||
None => Vec::new(),
|
||||
};
|
||||
assert_eq!(
|
||||
instruction_texts, expected_instruction_texts,
|
||||
"{case}: child received unexpected developer instructions"
|
||||
);
|
||||
assert!(
|
||||
child_texts.iter().all(|text| !text.is_empty()),
|
||||
"{case}: an empty developer fragment reached the model: {child_texts:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A full-history worker fork replaces parent instructions inside persisted compacted history.
|
||||
#[tokio::test]
|
||||
async fn compacted_full_history_fork_replaces_parent_developer_instructions() -> Result<()> {
|
||||
const COMPACT_SETUP_PROMPT: &str = "prepare the parent for compaction";
|
||||
const COMPACT_PROMPT: &str = "summarize the compacted parent";
|
||||
const COMPACTED_SUMMARY: &str = "preserved compacted parent summary";
|
||||
const SPAWN_PROMPT: &str = "spawn the compacted-history worker";
|
||||
const CHILD_PROMPT: &str = "inspect the compacted parent history";
|
||||
const SETUP_CALL_ID: &str = "trigger-parent-compaction";
|
||||
const SPAWN_CALL_ID: &str = "spawn-compacted-history-worker";
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let compaction_requests = responses::mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("parent-before-compaction"),
|
||||
responses::ev_function_call(SETUP_CALL_ID, "unsupported_tool", "{}"),
|
||||
responses::ev_completed_with_tokens(
|
||||
"parent-before-compaction",
|
||||
/*total_tokens*/ 96,
|
||||
),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("parent-compaction"),
|
||||
responses::ev_assistant_message("parent-summary", COMPACTED_SUMMARY),
|
||||
responses::ev_completed_with_tokens("parent-compaction", /*total_tokens*/ 10),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("parent-after-compaction"),
|
||||
responses::ev_assistant_message("parent-ready", "parent history compacted"),
|
||||
responses::ev_completed_with_tokens(
|
||||
"parent-after-compaction",
|
||||
/*total_tokens*/ 10,
|
||||
),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let parent_request = responses::mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| String::from_utf8_lossy(&request.body).contains(SPAWN_PROMPT),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("parent-spawn-after-compaction"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
SPAWN_CALL_ID,
|
||||
NAMESPACE,
|
||||
"spawn_agent",
|
||||
&serde_json::to_string(&json!({
|
||||
"message": CHILD_PROMPT,
|
||||
"task_name": "compacted_worker",
|
||||
}))?,
|
||||
),
|
||||
responses::ev_completed("parent-spawn-after-compaction"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let child_request = responses::mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| {
|
||||
let body = String::from_utf8_lossy(&request.body);
|
||||
body.contains(CHILD_PROMPT) && !body.contains(SPAWN_CALL_ID)
|
||||
},
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("compacted-child-work"),
|
||||
responses::ev_assistant_message("compacted-child-message", "child complete"),
|
||||
responses::ev_completed("compacted-child-work"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
responses::mount_sse_once_match(
|
||||
&server,
|
||||
|request: &wiremock::Request| {
|
||||
String::from_utf8_lossy(&request.body).contains(SPAWN_CALL_ID)
|
||||
},
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("compacted-parent-complete"),
|
||||
responses::ev_assistant_message("compacted-parent-message", "parent complete"),
|
||||
responses::ev_completed("compacted-parent-complete"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_model("gpt-5.4")
|
||||
.with_root_config(&format!(
|
||||
"developer_instructions = {PARENT_INSTRUCTIONS:?}\nmodel_context_window = 100\nmodel_auto_compact_token_limit = 90\ncompact_prompt = {COMPACT_PROMPT:?}"
|
||||
))
|
||||
.with_extra_config(&format!(
|
||||
"[features.multi_agent_v2]\nenabled = true\nsubagent_developer_instructions = {CHILD_INSTRUCTIONS:?}"
|
||||
))
|
||||
.write(codex_home.path())?;
|
||||
write_models_cache(codex_home.path())?;
|
||||
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } = app_server
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
READ_TIMEOUT,
|
||||
app_server.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: COMPACT_SETUP_PROMPT.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let compaction_requests = compaction_requests.requests();
|
||||
assert_eq!(compaction_requests.len(), 3);
|
||||
assert!(
|
||||
compaction_requests[1].body_contains_text(COMPACT_PROMPT),
|
||||
"the setup turn should perform actual mid-turn compaction"
|
||||
);
|
||||
assert!(
|
||||
compaction_requests[2]
|
||||
.message_input_texts("developer")
|
||||
.iter()
|
||||
.any(|text| text == PARENT_INSTRUCTIONS),
|
||||
"mid-turn compaction should retain parent instructions in its replacement history"
|
||||
);
|
||||
|
||||
let _: TurnStartResponse = app_server
|
||||
.request(|request_id| ClientRequest::TurnStart {
|
||||
request_id,
|
||||
params: TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
input: vec![UserInput::Text {
|
||||
text: SPAWN_PROMPT.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let child_request = timeout(READ_TIMEOUT, async {
|
||||
loop {
|
||||
if let Some(request) = child_request
|
||||
.requests()
|
||||
.into_iter()
|
||||
.find(|request| !request.inputs_of_type("agent_message").is_empty())
|
||||
{
|
||||
break request;
|
||||
}
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
parent_request
|
||||
.single_request()
|
||||
.message_input_texts("developer")
|
||||
.iter()
|
||||
.any(|text| text == PARENT_INSTRUCTIONS),
|
||||
"the parent should retain its own developer instructions after compaction"
|
||||
);
|
||||
assert!(
|
||||
child_request.body_contains_text(COMPACTED_SUMMARY),
|
||||
"the full-history child should inherit the compacted parent summary"
|
||||
);
|
||||
let child_developer_texts = child_request.message_input_texts("developer");
|
||||
assert_eq!(
|
||||
child_developer_texts
|
||||
.iter()
|
||||
.filter(|text| text.as_str() == CHILD_INSTRUCTIONS)
|
||||
.count(),
|
||||
1,
|
||||
"the child should receive its configured developer instructions exactly once"
|
||||
);
|
||||
assert!(
|
||||
child_developer_texts
|
||||
.iter()
|
||||
.all(|text| text != PARENT_INSTRUCTIONS),
|
||||
"the child should not inherit parent instructions from compacted history"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Cold root resume preserves inherited instructions or reapplies the configured v2 override.
|
||||
#[test_case(
|
||||
None,
|
||||
PARENT_INSTRUCTIONS,
|
||||
CHILD_INSTRUCTIONS;
|
||||
"inherits parent developer instructions without an override"
|
||||
)]
|
||||
#[test_case(
|
||||
Some(CHILD_INSTRUCTIONS),
|
||||
CHILD_INSTRUCTIONS,
|
||||
PARENT_INSTRUCTIONS;
|
||||
"reapplies configured subagent developer instructions"
|
||||
)]
|
||||
#[tokio::test]
|
||||
async fn cold_resume_preserves_effective_developer_instructions_for_roleless_worker(
|
||||
configured_subagent_developer_instructions: Option<&str>,
|
||||
expected_developer_instructions: &str,
|
||||
unexpected_developer_instructions: &str,
|
||||
) -> Result<()> {
|
||||
const INITIAL_PROMPT: &str = "spawn a durable instruction worker";
|
||||
const INITIAL_TASK: &str = "perform the initial durable instruction task";
|
||||
const FOLLOWUP_PROMPT: &str = "continue the durable instruction worker";
|
||||
@@ -33,7 +448,6 @@ async fn cold_resume_preserves_inherited_developer_instructions_for_roleless_wor
|
||||
const SPAWN_CALL_ID: &str = "spawn-durable-instruction-worker";
|
||||
const WAIT_CALL_ID: &str = "wait-for-durable-instruction-worker";
|
||||
const FOLLOWUP_CALL_ID: &str = "followup-durable-instruction-worker";
|
||||
const NAMESPACE: &str = "collaboration";
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
responses::mount_sse_once_match(
|
||||
@@ -94,11 +508,17 @@ async fn cold_resume_preserves_inherited_developer_instructions_for_roleless_wor
|
||||
)
|
||||
.await;
|
||||
|
||||
let feature_config = match configured_subagent_developer_instructions {
|
||||
Some(instructions) => format!(
|
||||
"[features.multi_agent_v2]\nenabled = true\nsubagent_developer_instructions = {instructions:?}"
|
||||
),
|
||||
None => "[features.multi_agent_v2]\nenabled = true".to_string(),
|
||||
};
|
||||
let codex_home = TempDir::new()?;
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_model("gpt-5.4")
|
||||
.with_root_config(&format!("developer_instructions = {PARENT_INSTRUCTIONS:?}"))
|
||||
.with_extra_config("[features.multi_agent_v2]\nenabled = true")
|
||||
.with_extra_config(&feature_config)
|
||||
.write(codex_home.path())?;
|
||||
write_models_cache(codex_home.path())?;
|
||||
|
||||
@@ -125,7 +545,6 @@ async fn cold_resume_preserves_inherited_developer_instructions_for_roleless_wor
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let initial_child_request = initial_child_request
|
||||
.requests()
|
||||
.into_iter()
|
||||
@@ -135,10 +554,15 @@ async fn cold_resume_preserves_inherited_developer_instructions_for_roleless_wor
|
||||
assert!(
|
||||
developer_texts
|
||||
.iter()
|
||||
.any(|text| text == PARENT_INSTRUCTIONS),
|
||||
"worker did not inherit parent developer instructions: {developer_texts:?}"
|
||||
.any(|text| text == expected_developer_instructions),
|
||||
"initial worker lost its effective developer instructions: {developer_texts:?}"
|
||||
);
|
||||
assert!(
|
||||
developer_texts
|
||||
.iter()
|
||||
.all(|text| text != unexpected_developer_instructions),
|
||||
"initial worker received the wrong developer instructions: {developer_texts:?}"
|
||||
);
|
||||
|
||||
let shutdown = timeout(READ_TIMEOUT, app_server.shutdown_gracefully()).await??;
|
||||
assert!(
|
||||
shutdown.success(),
|
||||
@@ -247,8 +671,14 @@ async fn cold_resume_preserves_inherited_developer_instructions_for_roleless_wor
|
||||
assert!(
|
||||
developer_texts
|
||||
.iter()
|
||||
.any(|text| text == PARENT_INSTRUCTIONS),
|
||||
"resumed worker lost its inherited developer instructions: {developer_texts:?}"
|
||||
.any(|text| text == expected_developer_instructions),
|
||||
"resumed worker lost its effective developer instructions: {developer_texts:?}"
|
||||
);
|
||||
assert!(
|
||||
developer_texts
|
||||
.iter()
|
||||
.all(|text| text != unexpected_developer_instructions),
|
||||
"resumed worker received the wrong developer instructions: {developer_texts:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -1851,6 +1851,10 @@
|
||||
"root_agent_usage_hint_text": {
|
||||
"type": "string"
|
||||
},
|
||||
"subagent_developer_instructions": {
|
||||
"description": "Overrides inherited developer instructions for subagents without role-specific instructions.",
|
||||
"type": "string"
|
||||
},
|
||||
"subagent_usage_hint_text": {
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::residency::is_v2_resident_session_source;
|
||||
use super::*;
|
||||
use crate::agent::role::apply_role_to_config;
|
||||
use crate::agent::role::apply_role_to_config_for_multi_agent_v2;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use codex_extension_api::ExtensionDataInit;
|
||||
|
||||
@@ -301,7 +301,7 @@ impl AgentControl {
|
||||
),
|
||||
};
|
||||
|
||||
apply_role_to_config(&mut config, Some(&role_name))
|
||||
apply_role_to_config_for_multi_agent_v2(&mut config, Some(&role_name))
|
||||
.await
|
||||
.map_err(CodexErr::InvalidRequest)?;
|
||||
config
|
||||
@@ -595,6 +595,32 @@ impl AgentControl {
|
||||
|
||||
let parent_thread_id = *parent_thread_id;
|
||||
let parent_thread = state.get_thread(parent_thread_id).await?;
|
||||
let (subagent_developer_instructions, parent_developer_instructions) = match (
|
||||
multi_agent_version,
|
||||
config
|
||||
.multi_agent_v2
|
||||
.subagent_developer_instructions
|
||||
.as_ref(),
|
||||
) {
|
||||
(MultiAgentVersion::V2, Some(_)) => {
|
||||
let parent_developer_instructions = match parent_thread
|
||||
.session
|
||||
.new_default_turn()
|
||||
.await
|
||||
.developer_instructions
|
||||
.clone()
|
||||
{
|
||||
Some(instructions) if !instructions.is_empty() => Some(instructions),
|
||||
Some(_) | None => None,
|
||||
};
|
||||
(
|
||||
Some(config.developer_instructions.clone().unwrap_or_default()),
|
||||
parent_developer_instructions,
|
||||
)
|
||||
}
|
||||
(MultiAgentVersion::Disabled | MultiAgentVersion::V1, _)
|
||||
| (MultiAgentVersion::V2, None) => (None, None),
|
||||
};
|
||||
let parent_history_mode = parent_thread.config_snapshot().await.history_mode;
|
||||
// `record_conversation_items` only queues persistence writes asynchronously.
|
||||
// Flush before snapshotting store history for a fork.
|
||||
@@ -644,43 +670,122 @@ impl AgentControl {
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let preserve_reference_context_item = matches!(fork_mode, SpawnAgentForkMode::FullHistory);
|
||||
forked_rollout_items.retain(|item| {
|
||||
keep_forked_rollout_item(item, preserve_reference_context_item)
|
||||
&& !matches!(
|
||||
item,
|
||||
RolloutItem::ResponseItem(response_item)
|
||||
if is_multi_agent_v2_usage_hint_message(
|
||||
response_item,
|
||||
&multi_agent_v2_usage_hint_texts_to_filter,
|
||||
)
|
||||
)
|
||||
});
|
||||
if destination_history_mode == Some(ThreadHistoryMode::Paginated) {
|
||||
forked_rollout_items.retain(|item| {
|
||||
!matches!(
|
||||
item,
|
||||
RolloutItem::EventMsg(
|
||||
EventMsg::ItemCompleted(_)
|
||||
| EventMsg::TokenCount(_)
|
||||
| EventMsg::ThreadGoalUpdated(_)
|
||||
| EventMsg::ThreadSettingsApplied(_),
|
||||
)
|
||||
)
|
||||
});
|
||||
}
|
||||
for item in &mut forked_rollout_items {
|
||||
if let RolloutItem::Compacted(compacted) = item
|
||||
&& let Some(replacement_history) = compacted.replacement_history.as_mut()
|
||||
{
|
||||
replacement_history.retain(|response_item| {
|
||||
!is_multi_agent_v2_usage_hint_message(
|
||||
response_item,
|
||||
&multi_agent_v2_usage_hint_texts_to_filter,
|
||||
)
|
||||
});
|
||||
let mut preserve_reference_context_item =
|
||||
matches!(fork_mode, SpawnAgentForkMode::FullHistory);
|
||||
if preserve_reference_context_item {
|
||||
for item in forked_rollout_items.iter().rev() {
|
||||
let RolloutItem::Compacted(compacted) = item else {
|
||||
continue;
|
||||
};
|
||||
// Legacy checkpoints force the child to rebuild context regardless of the
|
||||
// live parent's reference baseline; an older superseded checkpoint does not.
|
||||
if compacted.replacement_history.is_none() {
|
||||
preserve_reference_context_item = false;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
let mut replaced_parent_developer_instructions = false;
|
||||
// Scrub inherited hints and replace only the parent's developer-instruction fragment.
|
||||
// Compaction stores response items separately, so sanitize both top-level messages and
|
||||
// compacted replacement histories with the same policy.
|
||||
let retain_forked_item = |response_item: &mut ResponseItem, replaced: &mut bool| {
|
||||
if is_multi_agent_v2_usage_hint_message(
|
||||
response_item,
|
||||
&multi_agent_v2_usage_hint_texts_to_filter,
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if let Some(parent_developer_instructions) = parent_developer_instructions.as_ref()
|
||||
&& let Some(subagent_developer_instructions) =
|
||||
subagent_developer_instructions.as_ref()
|
||||
&& let ResponseItem::Message { role, content, .. } = response_item
|
||||
&& role == "developer"
|
||||
{
|
||||
content.retain_mut(|content_item| {
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
return true;
|
||||
};
|
||||
// TODO(anp) track better message fragment provenance in rollouts.
|
||||
if !text.contains(parent_developer_instructions) {
|
||||
return true;
|
||||
}
|
||||
|
||||
*replaced = true;
|
||||
let replacement = if preserve_reference_context_item {
|
||||
subagent_developer_instructions.as_str()
|
||||
} else {
|
||||
""
|
||||
};
|
||||
*text = text.replace(parent_developer_instructions, replacement);
|
||||
!text.is_empty()
|
||||
});
|
||||
return !content.is_empty();
|
||||
}
|
||||
|
||||
true
|
||||
};
|
||||
forked_rollout_items.retain_mut(|item| {
|
||||
if !keep_forked_rollout_item(item, preserve_reference_context_item)
|
||||
|| destination_history_mode == Some(ThreadHistoryMode::Paginated)
|
||||
&& matches!(
|
||||
&*item,
|
||||
RolloutItem::EventMsg(
|
||||
EventMsg::ItemCompleted(_)
|
||||
| EventMsg::TokenCount(_)
|
||||
| EventMsg::ThreadGoalUpdated(_)
|
||||
| EventMsg::ThreadSettingsApplied(_),
|
||||
)
|
||||
)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
match item {
|
||||
RolloutItem::ResponseItem(response_item) => {
|
||||
retain_forked_item(response_item, &mut replaced_parent_developer_instructions)
|
||||
}
|
||||
RolloutItem::Compacted(compacted) => {
|
||||
if let Some(replacement_history) = compacted.replacement_history.as_mut() {
|
||||
// Matches before this checkpoint cannot survive its replacement history.
|
||||
replaced_parent_developer_instructions = false;
|
||||
replacement_history.retain_mut(|response_item| {
|
||||
retain_forked_item(
|
||||
response_item,
|
||||
&mut replaced_parent_developer_instructions,
|
||||
)
|
||||
});
|
||||
}
|
||||
true
|
||||
}
|
||||
RolloutItem::EventMsg(_)
|
||||
| RolloutItem::SessionMeta(_)
|
||||
| RolloutItem::TurnContext(_)
|
||||
| RolloutItem::InterAgentCommunication(_)
|
||||
| RolloutItem::InterAgentCommunicationMetadata { .. }
|
||||
| RolloutItem::WorldState(_) => true,
|
||||
}
|
||||
});
|
||||
// Full forks reuse the parent's reference context instead of rebuilding it. If that
|
||||
// context omitted the parent's developer fragment, append the child's override so its
|
||||
// instructions still reach the model exactly once.
|
||||
if let Some(subagent_developer_instructions) = subagent_developer_instructions.as_ref()
|
||||
&& preserve_reference_context_item
|
||||
&& !replaced_parent_developer_instructions
|
||||
&& !subagent_developer_instructions.is_empty()
|
||||
&& parent_thread
|
||||
.session
|
||||
.reference_context_item()
|
||||
.await
|
||||
.is_some()
|
||||
&& let Some(developer_message) =
|
||||
crate::context_manager::updates::build_developer_update_item(vec![
|
||||
subagent_developer_instructions.clone(),
|
||||
])
|
||||
{
|
||||
forked_rollout_items.push(RolloutItem::ResponseItem(developer_message));
|
||||
}
|
||||
if preserve_reference_context_item
|
||||
&& multi_agent_version == MultiAgentVersion::V2
|
||||
&& let Some(subagent_usage_hint_text) =
|
||||
|
||||
@@ -1216,6 +1216,9 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
Some("Parent subagent guidance.".to_string());
|
||||
let mut child_config = harness.config.clone();
|
||||
let _ = child_config.features.enable(Feature::MultiAgentV2);
|
||||
child_config.developer_instructions = Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.root_agent_usage_hint_text =
|
||||
Some("Child root guidance.".to_string());
|
||||
child_config.multi_agent_v2.subagent_usage_hint_text =
|
||||
@@ -1275,7 +1278,8 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
role: "developer".to_string(),
|
||||
content: vec![
|
||||
ContentItem::InputText {
|
||||
text: "Parent developer instructions.".to_string(),
|
||||
text: "Developer context before.\nParent developer instructions.\nDeveloper context after."
|
||||
.to_string(),
|
||||
},
|
||||
ContentItem::InputText {
|
||||
text: "Preserved developer context.".to_string(),
|
||||
@@ -1353,7 +1357,8 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
role: "developer".to_string(),
|
||||
content: vec![
|
||||
ContentItem::InputText {
|
||||
text: "Parent developer instructions.".to_string(),
|
||||
text: "Developer context before.\nChild developer instructions.\nDeveloper context after."
|
||||
.to_string(),
|
||||
},
|
||||
ContentItem::InputText {
|
||||
text: "Preserved developer context.".to_string(),
|
||||
@@ -1392,6 +1397,10 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
|
||||
let mut no_hint_child_config = harness.config.clone();
|
||||
let _ = no_hint_child_config.features.enable(Feature::MultiAgentV2);
|
||||
no_hint_child_config.developer_instructions = Some(String::new());
|
||||
no_hint_child_config
|
||||
.multi_agent_v2
|
||||
.subagent_developer_instructions = Some(String::new());
|
||||
no_hint_child_config.multi_agent_v2.subagent_usage_hint_text = None;
|
||||
let no_hint_child_thread_id = harness
|
||||
.control
|
||||
@@ -1425,15 +1434,22 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
"full-history forked child should not add empty subagent guidance"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(
|
||||
!history_contains_text(
|
||||
no_hint_history.raw_items(),
|
||||
"Parent developer instructions."
|
||||
),
|
||||
"full-history forked child should inherit parent developer instructions"
|
||||
"empty child developer instructions should remove parent developer instructions"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(
|
||||
no_hint_history.raw_items(),
|
||||
"Developer context before.\n\nDeveloper context after."
|
||||
),
|
||||
"empty child developer instructions should preserve surrounding developer context"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(no_hint_history.raw_items(), "Preserved developer context."),
|
||||
"full-history forked child should preserve unrelated developer fragments"
|
||||
"empty child developer instructions should preserve unrelated developer fragments"
|
||||
);
|
||||
|
||||
let expected = (
|
||||
@@ -1484,6 +1500,9 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history() {
|
||||
Some("Parent subagent guidance.".to_string());
|
||||
let mut child_config = harness.config.clone();
|
||||
let _ = child_config.features.enable(Feature::MultiAgentV2);
|
||||
child_config.developer_instructions = Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.root_agent_usage_hint_text =
|
||||
Some("Child root guidance.".to_string());
|
||||
child_config.multi_agent_v2.subagent_usage_hint_text =
|
||||
@@ -1521,7 +1540,8 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history() {
|
||||
role: "developer".to_string(),
|
||||
content: vec![
|
||||
ContentItem::InputText {
|
||||
text: "Parent developer instructions.".to_string(),
|
||||
text: "Compacted context before.\nParent developer instructions.\nCompacted context after."
|
||||
.to_string(),
|
||||
},
|
||||
ContentItem::InputText {
|
||||
text: "Preserved compacted developer context.".to_string(),
|
||||
@@ -1590,8 +1610,15 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history() {
|
||||
"forked child history should strip stale parent hints from compacted replacement history"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "Parent developer instructions."),
|
||||
"forked child history should inherit compacted parent developer instructions"
|
||||
!history_contains_text(history.raw_items(), "Parent developer instructions."),
|
||||
"forked child history should replace parent instructions in compacted replacement history"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(
|
||||
history.raw_items(),
|
||||
"Compacted context before.\nChild developer instructions.\nCompacted context after."
|
||||
),
|
||||
"forked child history should replace compacted parent instructions without removing surrounding context"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(
|
||||
@@ -1616,6 +1643,301 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history() {
|
||||
.expect("parent shutdown should submit");
|
||||
}
|
||||
|
||||
/// Full-history forks must restore child instructions when compaction discarded
|
||||
/// the only matching parent instruction fragment from effective history.
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_full_fork_restores_instructions_after_compaction_discards_parent_fragment() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let mut parent_config = harness.config.clone();
|
||||
let _ = parent_config.features.enable(Feature::MultiAgentV2);
|
||||
parent_config.developer_instructions = Some("Parent developer instructions.".to_string());
|
||||
let mut child_config = parent_config.clone();
|
||||
child_config.developer_instructions = Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some("Child developer instructions.".to_string());
|
||||
|
||||
let new_thread = harness
|
||||
.manager
|
||||
.start_thread(StartThreadOptions::new(parent_config))
|
||||
.await
|
||||
.expect("start parent thread");
|
||||
let parent_thread_id = new_thread.thread_id;
|
||||
let parent_thread = new_thread.thread;
|
||||
let turn_context = parent_thread.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = "spawn-call-compacted-stale-instructions".to_string();
|
||||
let replacement_history = vec![
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "compacted parent summary".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "Preserved compacted developer context.".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
];
|
||||
|
||||
// Preserve the parent's live baseline while its durable checkpoint omits the
|
||||
// developer fragment that appeared in obsolete pre-compaction history.
|
||||
parent_thread
|
||||
.session
|
||||
.replace_history(
|
||||
replacement_history.clone(),
|
||||
Some(turn_context.to_turn_context_item()),
|
||||
)
|
||||
.await;
|
||||
parent_thread
|
||||
.session
|
||||
.persist_rollout_items(&[
|
||||
RolloutItem::ResponseItem(ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "Parent developer instructions.".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}),
|
||||
RolloutItem::Compacted(CompactedItem {
|
||||
message: String::new(),
|
||||
replacement_history: Some(replacement_history),
|
||||
window_number: None,
|
||||
first_window_id: None,
|
||||
previous_window_id: None,
|
||||
window_id: None,
|
||||
}),
|
||||
RolloutItem::TurnContext(turn_context.to_turn_context_item()),
|
||||
RolloutItem::ResponseItem(spawn_agent_call(&parent_spawn_call_id)),
|
||||
])
|
||||
.await;
|
||||
parent_thread.session.ensure_rollout_materialized().await;
|
||||
parent_thread
|
||||
.session
|
||||
.flush_rollout()
|
||||
.await
|
||||
.expect("parent rollout should flush");
|
||||
|
||||
let child_thread_id = harness
|
||||
.control
|
||||
.spawn_agent_with_metadata(
|
||||
child_config,
|
||||
text_input("child task"),
|
||||
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
})),
|
||||
SpawnAgentOptions {
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id),
|
||||
fork_mode: Some(SpawnAgentForkMode::FullHistory),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("forked spawn should preserve effective compacted instructions")
|
||||
.thread_id;
|
||||
let child_thread = harness
|
||||
.manager
|
||||
.get_thread(child_thread_id)
|
||||
.await
|
||||
.expect("child thread should be registered");
|
||||
let history = child_thread.session.clone_history().await;
|
||||
assert!(
|
||||
history_contains_text(
|
||||
history.raw_items(),
|
||||
"Preserved compacted developer context."
|
||||
),
|
||||
"full-history fork should preserve unrelated compacted developer fragments"
|
||||
);
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "Parent developer instructions."),
|
||||
"full-history fork should not restore stale pre-compaction parent instructions"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "Child developer instructions."),
|
||||
"full-history fork should append child instructions absent from effective compacted history"
|
||||
);
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
.shutdown_live_agent(child_thread_id)
|
||||
.await
|
||||
.expect("child shutdown should submit");
|
||||
let _ = parent_thread
|
||||
.submit(Op::Shutdown {})
|
||||
.await
|
||||
.expect("parent shutdown should submit");
|
||||
}
|
||||
|
||||
/// A legacy compaction clears the child's baseline, so its first turn must
|
||||
/// rebuild configured developer instructions exactly once.
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_once() {
|
||||
for (case, parent_developer_instructions) in [
|
||||
("without parent instructions", None),
|
||||
(
|
||||
"with parent instructions",
|
||||
Some("Parent developer instructions."),
|
||||
),
|
||||
] {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
let mut parent_config = harness.config.clone();
|
||||
let _ = parent_config.features.enable(Feature::MultiAgentV2);
|
||||
parent_config.developer_instructions = parent_developer_instructions.map(str::to_string);
|
||||
let mut child_config = parent_config.clone();
|
||||
child_config.developer_instructions = Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some("Child developer instructions.".to_string());
|
||||
|
||||
let new_thread = harness
|
||||
.manager
|
||||
.start_thread(StartThreadOptions::new(parent_config))
|
||||
.await
|
||||
.expect("start parent thread");
|
||||
let parent_thread_id = new_thread.thread_id;
|
||||
let parent_thread = new_thread.thread;
|
||||
let turn_context = parent_thread.session.new_default_turn().await;
|
||||
let parent_spawn_call_id = match parent_developer_instructions {
|
||||
Some(_) => "spawn-call-legacy-compact-with-parent",
|
||||
None => "spawn-call-legacy-compact-without-parent",
|
||||
};
|
||||
let parent_user_message = ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "parent task before legacy compaction".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
};
|
||||
|
||||
// A live parent can reestablish its baseline after resuming a rollout
|
||||
// whose older compaction record cannot restore that baseline to a child.
|
||||
parent_thread
|
||||
.session
|
||||
.replace_history(
|
||||
vec![parent_user_message.clone()],
|
||||
Some(turn_context.to_turn_context_item()),
|
||||
)
|
||||
.await;
|
||||
let mut rollout_items = vec![
|
||||
RolloutItem::ResponseItem(parent_user_message),
|
||||
RolloutItem::Compacted(CompactedItem {
|
||||
message: "legacy compacted summary".to_string(),
|
||||
replacement_history: None,
|
||||
window_number: None,
|
||||
first_window_id: None,
|
||||
previous_window_id: None,
|
||||
window_id: None,
|
||||
}),
|
||||
];
|
||||
if let Some(instructions) = parent_developer_instructions {
|
||||
rollout_items.push(RolloutItem::ResponseItem(ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: instructions.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}));
|
||||
}
|
||||
rollout_items.push(RolloutItem::TurnContext(
|
||||
turn_context.to_turn_context_item(),
|
||||
));
|
||||
rollout_items.push(RolloutItem::ResponseItem(spawn_agent_call(
|
||||
parent_spawn_call_id,
|
||||
)));
|
||||
parent_thread
|
||||
.session
|
||||
.persist_rollout_items(&rollout_items)
|
||||
.await;
|
||||
parent_thread.session.ensure_rollout_materialized().await;
|
||||
parent_thread
|
||||
.session
|
||||
.flush_rollout()
|
||||
.await
|
||||
.expect("parent rollout should flush");
|
||||
|
||||
let child_thread_id = harness
|
||||
.control
|
||||
.spawn_agent_with_metadata(
|
||||
child_config,
|
||||
text_input("child task"),
|
||||
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
})),
|
||||
SpawnAgentOptions {
|
||||
fork_parent_spawn_call_id: Some(parent_spawn_call_id.to_string()),
|
||||
fork_mode: Some(SpawnAgentForkMode::FullHistory),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("forked spawn should preserve legacy compacted history")
|
||||
.thread_id;
|
||||
let child_thread = harness
|
||||
.manager
|
||||
.get_thread(child_thread_id)
|
||||
.await
|
||||
.expect("child thread should be registered");
|
||||
while child_thread
|
||||
.session
|
||||
.reference_context_item()
|
||||
.await
|
||||
.is_none()
|
||||
{
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
let history = child_thread.session.clone_history().await;
|
||||
let mut instruction_count = 0;
|
||||
for item in history.raw_items() {
|
||||
let ResponseItem::Message { role, content, .. } = item else {
|
||||
continue;
|
||||
};
|
||||
if role != "developer" {
|
||||
continue;
|
||||
}
|
||||
for content_item in content {
|
||||
if let ContentItem::InputText { text } = content_item
|
||||
&& text == "Child developer instructions."
|
||||
{
|
||||
instruction_count += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
assert_eq!(
|
||||
instruction_count, 1,
|
||||
"{case}: canonical context reconstruction must not duplicate child developer instructions"
|
||||
);
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
.shutdown_live_agent(child_thread_id)
|
||||
.await
|
||||
.expect("child shutdown should submit");
|
||||
let _ = parent_thread
|
||||
.submit(Op::Shutdown {})
|
||||
.await
|
||||
.expect("parent shutdown should submit");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_fork_flushes_parent_rollout_before_loading_history() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
@@ -1934,6 +2256,9 @@ async fn spawn_agent_fork_last_n_turns_strips_parent_usage_hints() {
|
||||
Some("Parent root guidance.".to_string());
|
||||
let mut child_config = harness.config.clone();
|
||||
let _ = child_config.features.enable(Feature::MultiAgentV2);
|
||||
child_config.developer_instructions = Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some("Child developer instructions.".to_string());
|
||||
child_config.multi_agent_v2.subagent_usage_hint_text =
|
||||
Some("Child subagent guidance.".to_string());
|
||||
let new_thread = harness
|
||||
@@ -2024,8 +2349,12 @@ async fn spawn_agent_fork_last_n_turns_strips_parent_usage_hints() {
|
||||
"bounded fork should strip stale parent root hints before the child rebuilds startup context"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "Parent developer instructions."),
|
||||
"bounded fork should inherit parent developer instructions from retained turns"
|
||||
!history_contains_text(history.raw_items(), "Parent developer instructions."),
|
||||
"bounded fork should remove parent instructions before the child rebuilds startup context"
|
||||
);
|
||||
assert!(
|
||||
!history_contains_text(history.raw_items(), "Child developer instructions."),
|
||||
"bounded fork should not inject child instructions before its canonical context rebuild"
|
||||
);
|
||||
assert!(
|
||||
history_contains_text(history.raw_items(), "Preserved bounded developer context."),
|
||||
|
||||
@@ -38,6 +38,41 @@ const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not availabl
|
||||
pub(crate) async fn apply_role_to_config(
|
||||
config: &mut Config,
|
||||
role_name: Option<&str>,
|
||||
) -> Result<(), String> {
|
||||
apply_role_to_config_with_developer_instructions(
|
||||
config,
|
||||
role_name,
|
||||
RoleDeveloperInstructions::UseConfigLayers,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Applies a v2 role without losing developer instructions selected by its caller.
|
||||
///
|
||||
/// A role's own top-level developer instructions still take precedence. When its role file omits
|
||||
/// that setting, rebuilding the config must not restore inherited instructions from older layers.
|
||||
pub(crate) async fn apply_role_to_config_for_multi_agent_v2(
|
||||
config: &mut Config,
|
||||
role_name: Option<&str>,
|
||||
) -> Result<(), String> {
|
||||
apply_role_to_config_with_developer_instructions(
|
||||
config,
|
||||
role_name,
|
||||
RoleDeveloperInstructions::PreserveCallerInstructions,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum RoleDeveloperInstructions {
|
||||
UseConfigLayers,
|
||||
PreserveCallerInstructions,
|
||||
}
|
||||
|
||||
async fn apply_role_to_config_with_developer_instructions(
|
||||
config: &mut Config,
|
||||
role_name: Option<&str>,
|
||||
developer_instructions: RoleDeveloperInstructions,
|
||||
) -> Result<(), String> {
|
||||
let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME);
|
||||
|
||||
@@ -45,7 +80,7 @@ pub(crate) async fn apply_role_to_config(
|
||||
.cloned()
|
||||
.ok_or_else(|| format!("unknown agent_type '{role_name}'"))?;
|
||||
|
||||
apply_role_to_config_inner(config, role_name, &role)
|
||||
apply_role_to_config_inner(config, role_name, &role, developer_instructions)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
tracing::warn!("failed to apply role to config: {err}");
|
||||
@@ -57,6 +92,7 @@ async fn apply_role_to_config_inner(
|
||||
config: &mut Config,
|
||||
role_name: &str,
|
||||
role: &AgentRoleConfig,
|
||||
developer_instructions: RoleDeveloperInstructions,
|
||||
) -> anyhow::Result<()> {
|
||||
let is_built_in = !config.agent_roles.contains_key(role_name);
|
||||
let Some(config_file) = role.config_file.as_ref() else {
|
||||
@@ -75,6 +111,7 @@ async fn apply_role_to_config_inner(
|
||||
*config = reload::build_next_config(
|
||||
config,
|
||||
role_layer_toml,
|
||||
developer_instructions,
|
||||
preserve_current_provider,
|
||||
preserve_current_service_tier,
|
||||
)
|
||||
@@ -132,24 +169,35 @@ mod reload {
|
||||
pub(super) async fn build_next_config(
|
||||
config: &Config,
|
||||
role_layer_toml: TomlValue,
|
||||
developer_instructions: RoleDeveloperInstructions,
|
||||
preserve_current_provider: bool,
|
||||
preserve_current_service_tier: bool,
|
||||
) -> anyhow::Result<Config> {
|
||||
let preserve_current_model = role_layer_toml.get("model").is_none();
|
||||
let preserve_current_reasoning_effort =
|
||||
role_layer_toml.get("model_reasoning_effort").is_none();
|
||||
let mut overrides = reload_overrides(
|
||||
config,
|
||||
preserve_current_model,
|
||||
preserve_current_provider,
|
||||
preserve_current_service_tier,
|
||||
);
|
||||
if let (RoleDeveloperInstructions::PreserveCallerInstructions, Some(_), None) = (
|
||||
developer_instructions,
|
||||
&config.multi_agent_v2.subagent_developer_instructions,
|
||||
role_layer_toml.get("developer_instructions"),
|
||||
) {
|
||||
overrides
|
||||
.developer_instructions
|
||||
.clone_from(&config.developer_instructions);
|
||||
}
|
||||
let config_layer_stack = build_config_layer_stack(config, &role_layer_toml)?;
|
||||
let merged_config = deserialize_effective_config(config, &config_layer_stack)?;
|
||||
|
||||
let mut next_config = Config::load_config_with_layer_stack(
|
||||
LOCAL_FS.as_ref(),
|
||||
merged_config,
|
||||
reload_overrides(
|
||||
config,
|
||||
preserve_current_model,
|
||||
preserve_current_provider,
|
||||
preserve_current_service_tier,
|
||||
),
|
||||
overrides,
|
||||
config.codex_home.clone(),
|
||||
config_layer_stack,
|
||||
)
|
||||
|
||||
@@ -10762,6 +10762,7 @@ default_wait_timeout_ms = 30000
|
||||
usage_hint_text = "Custom delegation guidance."
|
||||
root_agent_usage_hint_text = "Root guidance."
|
||||
subagent_usage_hint_text = "Subagent guidance."
|
||||
subagent_developer_instructions = " Delegate carefully. "
|
||||
multi_agent_mode_hint_text = "Custom mode guidance."
|
||||
tool_namespace = "agents"
|
||||
hide_spawn_agent_metadata = true
|
||||
@@ -10804,6 +10805,13 @@ max_concurrent_threads_per_session = 9
|
||||
config.multi_agent_v2.subagent_usage_hint_text.as_deref(),
|
||||
Some("Subagent guidance.")
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.multi_agent_v2
|
||||
.subagent_developer_instructions
|
||||
.as_deref(),
|
||||
Some("Delegate carefully.")
|
||||
);
|
||||
assert_eq!(
|
||||
config.multi_agent_v2.multi_agent_mode_hint_text.as_deref(),
|
||||
Some("Custom mode guidance.")
|
||||
@@ -10965,11 +10973,13 @@ fn multi_agent_v2_preserves_empty_mode_hint_override() {
|
||||
let config_toml = toml::from_str(
|
||||
r#"[features.multi_agent_v2]
|
||||
multi_agent_mode_hint_text = ""
|
||||
subagent_developer_instructions = " \t "
|
||||
"#,
|
||||
)
|
||||
.expect("multi-agent v2 config should parse");
|
||||
|
||||
let expected = MultiAgentV2Config {
|
||||
subagent_developer_instructions: Some(String::new()),
|
||||
multi_agent_mode_hint_text: Some(String::new()),
|
||||
..resolve_multi_agent_v2_config(&ConfigToml::default())
|
||||
};
|
||||
|
||||
@@ -1250,6 +1250,7 @@ pub struct MultiAgentV2Config {
|
||||
pub usage_hint_text: Option<String>,
|
||||
pub root_agent_usage_hint_text: Option<String>,
|
||||
pub subagent_usage_hint_text: Option<String>,
|
||||
pub subagent_developer_instructions: Option<String>,
|
||||
pub multi_agent_mode_hint_text: Option<String>,
|
||||
pub tool_namespace: Option<String>,
|
||||
pub hide_spawn_agent_metadata: bool,
|
||||
@@ -1274,6 +1275,7 @@ impl MultiAgentV2Config {
|
||||
DEFAULT_MULTI_AGENT_V2_SUBAGENT_USAGE_HINT_TEXT,
|
||||
max_concurrent_threads_per_session,
|
||||
)),
|
||||
subagent_developer_instructions: None,
|
||||
multi_agent_mode_hint_text: None,
|
||||
tool_namespace: Some(DEFAULT_MULTI_AGENT_V2_TOOL_NAMESPACE.to_string()),
|
||||
hide_spawn_agent_metadata: true,
|
||||
@@ -2716,6 +2718,9 @@ fn resolve_multi_agent_v2_config(config_toml: &ConfigToml) -> MultiAgentV2Config
|
||||
base.map(|config| &config.subagent_usage_hint_text),
|
||||
default_subagent_usage_hint_text,
|
||||
);
|
||||
let subagent_developer_instructions = base
|
||||
.and_then(|config| config.subagent_developer_instructions.as_ref())
|
||||
.map(|instructions| instructions.trim().to_string());
|
||||
let multi_agent_mode_hint_text = base
|
||||
.and_then(|config| config.multi_agent_mode_hint_text.as_ref())
|
||||
.cloned()
|
||||
@@ -2736,6 +2741,7 @@ fn resolve_multi_agent_v2_config(config_toml: &ConfigToml) -> MultiAgentV2Config
|
||||
usage_hint_text,
|
||||
root_agent_usage_hint_text,
|
||||
subagent_usage_hint_text,
|
||||
subagent_developer_instructions,
|
||||
multi_agent_mode_hint_text,
|
||||
tool_namespace,
|
||||
hide_spawn_agent_metadata,
|
||||
|
||||
@@ -262,6 +262,8 @@ mod tests {
|
||||
async fn lock_contains_prompts_and_materializes_features() {
|
||||
let mut sc = crate::session::tests::make_session_configuration_for_tests().await;
|
||||
let mut config = (*sc.original_config_do_not_use).clone();
|
||||
config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some("Locked subagent developer instructions.".to_string());
|
||||
config.token_budget = Some(crate::config::TokenBudgetConfig {
|
||||
reminder_threshold_tokens: Some(16_000),
|
||||
reminder_message_template: "Locked reminder: {n_remaining} tokens.".to_string(),
|
||||
@@ -346,9 +348,10 @@ mod tests {
|
||||
min_wait_timeout_ms: Some(_),
|
||||
max_wait_timeout_ms: Some(_),
|
||||
default_wait_timeout_ms: Some(_),
|
||||
subagent_developer_instructions: Some(instructions),
|
||||
hide_spawn_agent_metadata: Some(_),
|
||||
..
|
||||
})
|
||||
}) if instructions == "Locked subagent developer instructions."
|
||||
));
|
||||
|
||||
assert_eq!(
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::agent::role::apply_role_to_config;
|
||||
use crate::agent::role::apply_role_to_config_for_multi_agent_v2;
|
||||
use crate::config::Config;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::config::HARD_MAX_MULTI_AGENT_V2_TIMEOUT_MS;
|
||||
@@ -200,6 +201,15 @@ fn build_agent_shared_config(turn: &TurnContext) -> Result<Config, FunctionCallE
|
||||
.or_else(|| turn.model_info.default_reasoning_level.clone());
|
||||
config.model_reasoning_summary = Some(turn.reasoning_summary);
|
||||
config.developer_instructions = turn.developer_instructions.clone();
|
||||
if turn.multi_agent_version == MultiAgentVersion::V2
|
||||
&& let Some(developer_instructions) = turn
|
||||
.config
|
||||
.multi_agent_v2
|
||||
.subagent_developer_instructions
|
||||
.clone()
|
||||
{
|
||||
config.developer_instructions = Some(developer_instructions);
|
||||
}
|
||||
apply_spawn_agent_runtime_overrides(&mut config, turn)?;
|
||||
|
||||
Ok(config)
|
||||
@@ -364,9 +374,15 @@ pub(crate) async fn apply_spawn_agent_role(
|
||||
) -> Result<(), FunctionCallError> {
|
||||
let previous_model = config.model.clone();
|
||||
let previous_reasoning_effort = config.model_reasoning_effort.clone();
|
||||
apply_role_to_config(config, role_name)
|
||||
.await
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
if session.multi_agent_version() == Some(MultiAgentVersion::V2) {
|
||||
apply_role_to_config_for_multi_agent_v2(config, role_name)
|
||||
.await
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
} else {
|
||||
apply_role_to_config(config, role_name)
|
||||
.await
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
}
|
||||
if config.model == previous_model && config.model_reasoning_effort == previous_reasoning_effort
|
||||
{
|
||||
return Ok(());
|
||||
|
||||
@@ -30,6 +30,7 @@ const ROLE_NAME: &str = "durable_worker";
|
||||
const ROLE_MODEL: &str = "gpt-5.4";
|
||||
const ROLE_MODEL_PROVIDER_ID: &str = "mock";
|
||||
const ROLE_DEVELOPER_INSTRUCTIONS: &str = "Keep the durable worker role configuration.";
|
||||
const SUBAGENT_DEVELOPER_INSTRUCTIONS: &str = "Use the default durable worker instructions.";
|
||||
|
||||
fn decoded_body(request: &wiremock::Request) -> Option<Vec<u8>> {
|
||||
let is_zstd = request
|
||||
@@ -77,6 +78,8 @@ fn configure_multi_agent_v2_with_role(
|
||||
.features
|
||||
.enable(Feature::MultiAgentV2)
|
||||
.expect("test config should allow feature update");
|
||||
config.multi_agent_v2.subagent_developer_instructions =
|
||||
Some(SUBAGENT_DEVELOPER_INSTRUCTIONS.to_string());
|
||||
let role_path = config.codex_home.join("durable-worker-role.toml");
|
||||
std::fs::write(
|
||||
&role_path,
|
||||
@@ -192,6 +195,7 @@ async fn cold_root_resume_restores_agent_identity_and_role_on_followup() -> Resu
|
||||
assert!(initial_child_request.requests().iter().any(|request| {
|
||||
request.body_contains_text(INITIAL_TASK)
|
||||
&& request.body_contains_text(ROLE_DEVELOPER_INSTRUCTIONS)
|
||||
&& !request.body_contains_text(SUBAGENT_DEVELOPER_INSTRUCTIONS)
|
||||
}));
|
||||
let initial_worker_config = worker_thread.config_snapshot().await;
|
||||
let initial_worker_role_config = (
|
||||
@@ -300,6 +304,7 @@ async fn cold_root_resume_restores_agent_identity_and_role_on_followup() -> Resu
|
||||
assert!(followup_child_request.requests().iter().any(|request| {
|
||||
request.body_contains_text(FOLLOWUP_TASK)
|
||||
&& request.body_contains_text(ROLE_DEVELOPER_INSTRUCTIONS)
|
||||
&& !request.body_contains_text(SUBAGENT_DEVELOPER_INSTRUCTIONS)
|
||||
}));
|
||||
let reloaded_worker_config = reloaded_worker.config_snapshot().await;
|
||||
let reloaded_worker_role_config = (
|
||||
|
||||
@@ -95,6 +95,9 @@ pub struct MultiAgentV2ConfigToml {
|
||||
pub root_agent_usage_hint_text: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub subagent_usage_hint_text: Option<String>,
|
||||
/// Overrides inherited developer instructions for subagents without role-specific instructions.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub subagent_developer_instructions: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub multi_agent_mode_hint_text: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -444,6 +444,7 @@ usage_hint_enabled = false
|
||||
usage_hint_text = "Custom delegation guidance."
|
||||
root_agent_usage_hint_text = "Root guidance."
|
||||
subagent_usage_hint_text = "Subagent guidance."
|
||||
subagent_developer_instructions = "Delegate carefully."
|
||||
multi_agent_mode_hint_text = "Custom mode guidance."
|
||||
tool_namespace = "agents"
|
||||
hide_spawn_agent_metadata = true
|
||||
@@ -470,6 +471,7 @@ non_code_mode_only = true
|
||||
usage_hint_text: Some("Custom delegation guidance.".to_string()),
|
||||
root_agent_usage_hint_text: Some("Root guidance.".to_string()),
|
||||
subagent_usage_hint_text: Some("Subagent guidance.".to_string()),
|
||||
subagent_developer_instructions: Some("Delegate carefully.".to_string()),
|
||||
multi_agent_mode_hint_text: Some("Custom mode guidance.".to_string()),
|
||||
tool_namespace: Some("agents".to_string()),
|
||||
hide_spawn_agent_metadata: Some(true),
|
||||
@@ -538,6 +540,7 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
multi_agent_v2: Some(FeatureToml::Config(crate::MultiAgentV2ConfigToml {
|
||||
enabled: Some(false),
|
||||
min_wait_timeout_ms: Some(2500),
|
||||
subagent_developer_instructions: Some("Delegate carefully.".to_string()),
|
||||
..Default::default()
|
||||
})),
|
||||
network_proxy: Some(FeatureToml::Config(crate::NetworkProxyConfigToml {
|
||||
@@ -578,6 +581,7 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
Some(FeatureToml::Config(crate::MultiAgentV2ConfigToml {
|
||||
enabled: Some(true),
|
||||
min_wait_timeout_ms: Some(2500),
|
||||
subagent_developer_instructions: Some("Delegate carefully.".to_string()),
|
||||
..Default::default()
|
||||
}))
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user