mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Allow agent roles on full-history forks (#37252)
## Why Multi-agent v2 rejected `agent_type` when `fork_turns` was `all`, forcing callers to choose between a configured role and the parent's full conversation context. ## What changed - Apply an explicitly selected role to full-history forks, including its model and developer instructions. - Preserve the parent's developer instructions when the selected role does not define its own. - Keep the inherited conversation history and model-switch context intact. - Update the `spawn_agent` schema to clarify that roles apply independently of history inheritance. ## Testing - Cover full-history role overrides and developer-instruction precedence in multi-agent v2 tests. GitOrigin-RevId: cd4f35e31d75f56289760f245bff38dd0233abe6
This commit is contained in:
@@ -36,6 +36,7 @@ const ROLE_INSTRUCTIONS: &str = "configured role developer instructions";
|
||||
#[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("full history configured role"; "full history configured role")]
|
||||
#[test_case("implicit configured default"; "implicit configured default")]
|
||||
#[test_case("full fork skips default role"; "full fork skips default role")]
|
||||
#[tokio::test]
|
||||
@@ -44,18 +45,19 @@ async fn spawned_subagents_apply_configured_developer_instruction_precedence(
|
||||
) -> Result<()> {
|
||||
let fork_turns = match case {
|
||||
"bounded history" => Some("1"),
|
||||
"no history"
|
||||
| "configured role without instructions"
|
||||
| "explicit configured role"
|
||||
| "implicit configured default" => Some("none"),
|
||||
"no history" | "explicit configured role" | "implicit configured default" => Some("none"),
|
||||
_ => None,
|
||||
};
|
||||
let agent_type = match case {
|
||||
"configured role without instructions" | "explicit configured role" => Some("custom"),
|
||||
"configured role without instructions"
|
||||
| "explicit configured role"
|
||||
| "full history configured role" => Some("custom"),
|
||||
_ => None,
|
||||
};
|
||||
let configured_override = match case {
|
||||
"unset override" => None,
|
||||
"unset override"
|
||||
| "full history configured role"
|
||||
| "configured role without instructions" => None,
|
||||
"blank override" => Some(" "),
|
||||
"full history" => Some(" child-only developer instructions "),
|
||||
_ => Some(CHILD_INSTRUCTIONS),
|
||||
@@ -69,17 +71,23 @@ async fn spawned_subagents_apply_configured_developer_instruction_precedence(
|
||||
case,
|
||||
"configured role without instructions"
|
||||
| "explicit configured role"
|
||||
| "full history 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"
|
||||
"explicit configured role"
|
||||
| "full history configured role"
|
||||
| "implicit configured default"
|
||||
| "full fork skips default role"
|
||||
);
|
||||
let expected = match case {
|
||||
"unset override" => Some(PARENT_INSTRUCTIONS),
|
||||
"unset override" | "configured role without instructions" => Some(PARENT_INSTRUCTIONS),
|
||||
"blank override" => None,
|
||||
"explicit configured role" | "implicit configured default" => Some(ROLE_INSTRUCTIONS),
|
||||
"explicit configured role"
|
||||
| "full history configured role"
|
||||
| "implicit configured default" => Some(ROLE_INSTRUCTIONS),
|
||||
_ => Some(CHILD_INSTRUCTIONS),
|
||||
};
|
||||
const PARENT_PROMPT: &str = "spawn the instruction override worker";
|
||||
@@ -149,7 +157,12 @@ async fn spawned_subagents_apply_configured_developer_instruction_precedence(
|
||||
);
|
||||
}
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut config = MockResponsesConfig::new(&server.uri()).with_model("gpt-5.4");
|
||||
let configured_model = if case == "full history configured role" {
|
||||
"gpt-5.5"
|
||||
} else {
|
||||
"gpt-5.4"
|
||||
};
|
||||
let mut config = MockResponsesConfig::new(&server.uri()).with_model(configured_model);
|
||||
if role_has_instructions {
|
||||
config =
|
||||
config.with_root_config(&format!("developer_instructions = {ROLE_INSTRUCTIONS:?}"));
|
||||
@@ -206,6 +219,19 @@ async fn spawned_subagents_apply_configured_developer_instruction_precedence(
|
||||
);
|
||||
}
|
||||
let child_texts = child_request.message_input_texts("developer");
|
||||
if case == "full history configured role" {
|
||||
assert_eq!(child_request.body_json()["model"], json!("gpt-5.5"));
|
||||
assert!(
|
||||
child_request.body_contains_text(PARENT_PROMPT),
|
||||
"the child should inherit the parent's conversation history"
|
||||
);
|
||||
assert!(
|
||||
child_texts
|
||||
.iter()
|
||||
.any(|text| text.contains("<model_switch>")),
|
||||
"the child should preserve the parent's model context"
|
||||
);
|
||||
}
|
||||
let instruction_texts = child_texts
|
||||
.iter()
|
||||
.map(String::as_str)
|
||||
|
||||
@@ -625,7 +625,9 @@ impl AgentControl {
|
||||
.subagent_developer_instructions
|
||||
.as_ref(),
|
||||
) {
|
||||
(MultiAgentVersion::V2, Some(_)) => {
|
||||
(MultiAgentVersion::V2, override_instructions)
|
||||
if override_instructions.is_some() || session_source.get_agent_role().is_some() =>
|
||||
{
|
||||
let parent_developer_instructions = match parent_thread
|
||||
.session
|
||||
.new_default_turn()
|
||||
@@ -642,7 +644,7 @@ impl AgentControl {
|
||||
)
|
||||
}
|
||||
(MultiAgentVersion::Disabled | MultiAgentVersion::V1, _)
|
||||
| (MultiAgentVersion::V2, None) => (None, None),
|
||||
| (MultiAgentVersion::V2, _) => (None, None),
|
||||
};
|
||||
let parent_history_mode = parent_thread.config_snapshot().await.history_mode;
|
||||
// `record_conversation_items` only queues persistence writes asynchronously.
|
||||
|
||||
@@ -640,7 +640,7 @@ fn spawn_agent_common_properties_v2(agent_type_description: &str) -> BTreeMap<St
|
||||
(
|
||||
"agent_type".to_string(),
|
||||
JsonSchema::string(Some(format!(
|
||||
"Agent type override for the new agent. Omit unless explicitly asked. Set `fork_turns` to `none` or a positive integer when an explicit override is needed.\n{agent_type_description}"
|
||||
"Agent type override for the new agent. Omit unless explicitly asked. The selected role applies regardless of how much parent history is inherited.\n{agent_type_description}"
|
||||
))),
|
||||
),
|
||||
(
|
||||
|
||||
@@ -346,7 +346,7 @@ async fn spawn_agent_fork_context_rejects_agent_type_override() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() {
|
||||
async fn multi_agent_v2_spawn_fork_turns_all_applies_agent_type_override() {
|
||||
let (mut session, mut turn) = make_session_and_context().await;
|
||||
let role_name = install_role_with_model_override(&mut turn).await;
|
||||
let manager = thread_manager();
|
||||
@@ -367,7 +367,7 @@ async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() {
|
||||
..turn
|
||||
};
|
||||
|
||||
let err = SpawnAgentHandlerV2::default()
|
||||
SpawnAgentHandlerV2::default()
|
||||
.handle(invocation(
|
||||
Arc::new(session),
|
||||
Arc::new(turn),
|
||||
@@ -380,15 +380,7 @@ async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() {
|
||||
})),
|
||||
))
|
||||
.await
|
||||
.err()
|
||||
.expect("fork_turns=all should reject agent_type overrides");
|
||||
|
||||
assert_eq!(
|
||||
err,
|
||||
FunctionCallError::RespondToModel(
|
||||
"Full-history forked agents inherit the parent agent type; omit agent_type, or spawn without a full-history fork.".to_string(),
|
||||
)
|
||||
);
|
||||
.expect("fork_turns=all should apply agent_type overrides");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -69,9 +69,6 @@ async fn handle_spawn_agent(
|
||||
config.service_tier = Some(service_tier.clone());
|
||||
}
|
||||
let is_full_history_fork = matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory));
|
||||
if is_full_history_fork {
|
||||
reject_full_fork_agent_type_override(role_name)?;
|
||||
}
|
||||
apply_requested_spawn_agent_model_overrides(
|
||||
&session,
|
||||
turn.as_ref(),
|
||||
@@ -80,8 +77,13 @@ async fn handle_spawn_agent(
|
||||
args.reasoning_effort.clone(),
|
||||
)
|
||||
.await?;
|
||||
if !is_full_history_fork {
|
||||
if !is_full_history_fork || role_name.is_some() {
|
||||
apply_spawn_agent_role(&session, &mut config, role_name).await?;
|
||||
if is_full_history_fork && config.developer_instructions.is_none() {
|
||||
config
|
||||
.developer_instructions
|
||||
.clone_from(&turn.developer_instructions);
|
||||
}
|
||||
}
|
||||
apply_spawn_agent_service_tier(
|
||||
&session,
|
||||
|
||||
Reference in New Issue
Block a user