Merge remote-tracking branch 'origin/dev/friel/fork-context-inherits-parent-model' into repair/watchdog-prompt-layering

This commit is contained in:
Friel
2026-03-29 18:07:01 +00:00
12 changed files with 183 additions and 22 deletions
+4
View File
@@ -21,6 +21,10 @@
"description": "Human-facing role documentation used in spawn tool guidance. Required unless supplied by the referenced agent role file.",
"type": "string"
},
"fork_context": {
"description": "Default fork-context behavior for this role.",
"type": "boolean"
},
"nickname_candidates": {
"description": "Candidate nicknames for agents spawned with this role.",
"items": {
+1
View File
@@ -1549,6 +1549,7 @@ async fn spawn_thread_subagent_uses_role_specific_nickname_candidates() {
description: Some("Research role".to_string()),
config_file: None,
nickname_candidates: Some(vec!["Atlas".to_string()]),
fork_context: None,
},
);
let (parent_thread_id, _parent_thread) = harness.start_thread().await;
+10
View File
@@ -119,6 +119,13 @@ pub(crate) fn resolve_role_config<'a>(
.or_else(|| built_in::configs().get(role_name))
}
pub(crate) fn default_fork_context_for_role(config: &Config, role_name: Option<&str>) -> bool {
let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME);
resolve_role_config(config, role_name)
.and_then(|role| role.fork_context)
.unwrap_or(false)
}
fn preservation_policy(config: &Config, role_layer_toml: &TomlValue) -> (bool, bool) {
let role_selects_provider = role_layer_toml.get("model_provider").is_some();
let role_selects_profile = role_layer_toml.get("profile").is_some();
@@ -352,6 +359,7 @@ mod built_in {
description: Some("Default agent.".to_string()),
config_file: None,
nickname_candidates: None,
fork_context: Some(true),
}
),
(
@@ -366,6 +374,7 @@ Rules:
- Reuse existing explorers for related questions."#.to_string()),
config_file: Some("explorer.toml".to_string().parse().unwrap_or_default()),
nickname_candidates: None,
fork_context: Some(true),
}
),
(
@@ -381,6 +390,7 @@ Rules:
- Always tell workers they are **not alone in the codebase**, and they should not revert the edits made by others, and they should adjust their implementation to accommodate the changes made by others. This is important because there may be multiple workers making changes in parallel, and they need to be aware of each other's work to avoid conflicts and ensure a cohesive final product."#.to_string()),
config_file: None,
nickname_candidates: None,
fork_context: Some(true),
}
),
// Awaiter is temp removed
+16
View File
@@ -96,6 +96,7 @@ async fn apply_role_returns_unavailable_for_missing_user_role_file() {
description: None,
config_file: Some(PathBuf::from("/path/does/not/exist.toml")),
nickname_candidates: None,
fork_context: None,
},
);
@@ -116,6 +117,7 @@ async fn apply_role_returns_unavailable_for_invalid_user_role_toml() {
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -147,6 +149,7 @@ model = "role-model"
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -178,6 +181,7 @@ async fn apply_role_preserves_unspecified_keys() {
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -237,6 +241,7 @@ model_provider = "test-provider"
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -291,6 +296,7 @@ model_verbosity = "high"
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -357,6 +363,7 @@ model_provider = "role-provider"
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -415,6 +422,7 @@ model_provider = "base-provider"
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -479,6 +487,7 @@ model_reasoning_effort = "high"
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -523,6 +532,7 @@ writable_roots = ["./sandbox-root"]
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -585,6 +595,7 @@ async fn apply_role_takes_precedence_over_existing_session_flags_for_same_key()
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -628,6 +639,7 @@ enabled = false
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
@@ -662,6 +674,7 @@ fn spawn_tool_spec_build_deduplicates_user_defined_built_in_roles() {
description: Some("user override".to_string()),
config_file: None,
nickname_candidates: None,
fork_context: None,
},
),
("researcher".to_string(), AgentRoleConfig::default()),
@@ -683,6 +696,7 @@ fn spawn_tool_spec_lists_user_defined_roles_before_built_ins() {
description: Some("first".to_string()),
config_file: None,
nickname_candidates: None,
fork_context: None,
},
)]);
@@ -710,6 +724,7 @@ fn spawn_tool_spec_marks_role_locked_model_and_reasoning_effort() {
description: Some("Research carefully.".to_string()),
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
)]);
@@ -735,6 +750,7 @@ fn spawn_tool_spec_marks_role_locked_reasoning_effort_only() {
description: Some("Review carefully.".to_string()),
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
)]);
+1
View File
@@ -2369,6 +2369,7 @@ enabled = false
description: None,
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
crate::agent::role::apply_role_to_config(&mut child_config, Some("custom"))
+3
View File
@@ -157,6 +157,7 @@ fn merge_missing_role_fields(role: &mut AgentRoleConfig, fallback: &AgentRoleCon
.nickname_candidates
.clone()
.or(fallback.nickname_candidates.clone());
role.fork_context = role.fork_context.or(fallback.fork_context);
}
fn agents_toml_from_layer(layer_toml: &TomlValue) -> std::io::Result<Option<AgentsToml>> {
@@ -190,6 +191,7 @@ fn agent_role_config_from_toml(
description,
config_file,
nickname_candidates,
fork_context: role.fork_context,
})
}
@@ -480,6 +482,7 @@ fn discover_agent_roles_in_dir(
description: parsed_file.description,
config_file: Some(agent_file),
nickname_candidates: parsed_file.nickname_candidates,
fork_context: None,
},
);
}
+5
View File
@@ -3219,6 +3219,7 @@ fn load_config_rejects_missing_agent_role_config_file() -> std::io::Result<()> {
description: Some("Research role".to_string()),
config_file: Some(missing_path.abs()),
nickname_candidates: None,
fork_context: None,
},
)]),
}),
@@ -4087,6 +4088,7 @@ fn load_config_normalizes_agent_role_nickname_candidates() -> std::io::Result<()
" Hypatia ".to_string(),
"Noether".to_string(),
]),
fork_context: None,
},
)]),
}),
@@ -4125,6 +4127,7 @@ fn load_config_rejects_empty_agent_role_nickname_candidates() -> std::io::Result
description: Some("Research role".to_string()),
config_file: None,
nickname_candidates: Some(Vec::new()),
fork_context: None,
},
)]),
}),
@@ -4160,6 +4163,7 @@ fn load_config_rejects_duplicate_agent_role_nickname_candidates() -> std::io::Re
description: Some("Research role".to_string()),
config_file: None,
nickname_candidates: Some(vec!["Hypatia".to_string(), " Hypatia ".to_string()]),
fork_context: None,
},
)]),
}),
@@ -4195,6 +4199,7 @@ fn load_config_rejects_unsafe_agent_role_nickname_candidates() -> std::io::Resul
description: Some("Research role".to_string()),
config_file: None,
nickname_candidates: Some(vec!["Agent <One>".to_string()]),
fork_context: None,
},
)]),
}),
+5
View File
@@ -1575,6 +1575,8 @@ pub struct AgentRoleConfig {
pub config_file: Option<PathBuf>,
/// Candidate nicknames for agents spawned with this role.
pub nickname_candidates: Option<Vec<String>>,
/// Default fork-context behavior for this role.
pub fork_context: Option<bool>,
}
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
@@ -1590,6 +1592,9 @@ pub struct AgentRoleToml {
/// Candidate nicknames for agents spawned with this role.
pub nickname_candidates: Option<Vec<String>>,
/// Default fork-context behavior for this role.
pub fork_context: Option<bool>,
}
impl From<ToolsToml> for Tools {
@@ -3,6 +3,7 @@ use crate::agent::control::SpawnAgentOptions;
use crate::agent::control::render_input_preview;
use crate::agent::role::DEFAULT_ROLE_NAME;
use crate::agent::role::apply_role_to_config;
use crate::agent::role::default_fork_context_for_role;
use crate::agent::exceeds_thread_spawn_depth_limit;
use crate::agent::next_thread_spawn_depth;
@@ -61,14 +62,19 @@ impl ToolHandler for Handler {
.await;
let mut config =
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
apply_requested_spawn_agent_model_overrides(
&session,
turn.as_ref(),
&mut config,
args.model.as_deref(),
args.reasoning_effort,
)
.await?;
let fork_context = args
.fork_context
.unwrap_or_else(|| default_fork_context_for_role(&turn.config, role_name));
if !fork_context {
apply_requested_spawn_agent_model_overrides(
&session,
turn.as_ref(),
&mut config,
args.model.as_deref(),
args.reasoning_effort,
)
.await?;
}
apply_role_to_config(&mut config, role_name)
.await
.map_err(FunctionCallError::RespondToModel)?;
@@ -89,7 +95,7 @@ impl ToolHandler for Handler {
/*task_name*/ None,
)?),
SpawnAgentOptions {
fork_parent_spawn_call_id: args.fork_context.then(|| call_id.clone()),
fork_parent_spawn_call_id: fork_context.then(|| call_id.clone()),
},
)
.await
@@ -174,8 +180,7 @@ struct SpawnAgentArgs {
agent_type: Option<String>,
model: Option<String>,
reasoning_effort: Option<ReasoningEffort>,
#[serde(default)]
fork_context: bool,
fork_context: Option<bool>,
}
#[derive(Debug, Serialize)]
@@ -295,6 +295,110 @@ async fn spawn_agent_uses_explorer_role_and_preserves_approval_policy() {
assert_eq!(snapshot.model_provider_id, "ollama");
}
#[tokio::test]
async fn spawn_agent_fork_context_ignores_child_model_overrides() {
let (mut session, turn) = make_session_and_context().await;
let manager = thread_manager();
let root = manager
.start_thread((*turn.config).clone())
.await
.expect("root thread should start");
session.services.agent_control = manager.agent_control();
session.conversation_id = root.thread_id;
let expected_model = turn.model_info.slug.clone();
let expected_reasoning_effort = turn.reasoning_effort;
let output = SpawnAgentHandler
.handle(invocation(
Arc::new(session),
Arc::new(turn),
"spawn_agent",
function_payload(json!({
"message": "inspect this repo",
"model": "not-a-real-model",
"reasoning_effort": "low",
"fork_context": true
})),
))
.await
.expect("spawn_agent should succeed");
let (content, _) = expect_text_output(output);
let result: serde_json::Value =
serde_json::from_str(&content).expect("spawn_agent result should be json");
let agent_id = parse_agent_id(
result["agent_id"]
.as_str()
.expect("spawn_agent result should include agent_id"),
);
let snapshot = manager
.get_thread(agent_id)
.await
.expect("spawned agent thread should exist")
.config_snapshot()
.await;
assert_eq!(snapshot.model, expected_model);
assert_eq!(snapshot.reasoning_effort, expected_reasoning_effort);
}
#[tokio::test]
async fn multi_agent_v2_spawn_fork_context_ignores_child_model_overrides() {
let (mut session, turn) = make_session_and_context().await;
let manager = thread_manager();
let root = manager
.start_thread((*turn.config).clone())
.await
.expect("root thread should start");
session.services.agent_control = manager.agent_control();
session.conversation_id = root.thread_id;
let mut config = (*turn.config).clone();
config
.features
.enable(Feature::MultiAgentV2)
.expect("test config should allow feature update");
let turn = TurnContext {
config: Arc::new(config),
..turn
};
let expected_model = turn.model_info.slug.clone();
let expected_reasoning_effort = turn.reasoning_effort;
let output = SpawnAgentHandlerV2
.handle(invocation(
Arc::new(session),
Arc::new(turn),
"spawn_agent",
function_payload(json!({
"message": "inspect this repo",
"model": "not-a-real-model",
"reasoning_effort": "low",
"fork_context": true,
"task_name": "fork_context_v2"
})),
))
.await
.expect("spawn_agent should succeed");
let (content, _) = expect_text_output(output);
let result: serde_json::Value =
serde_json::from_str(&content).expect("spawn_agent result should be json");
assert_eq!(result["task_name"], "/root/fork_context_v2");
let agent_id = manager
.captured_ops()
.into_iter()
.map(|(thread_id, _)| thread_id)
.find(|thread_id| *thread_id != root.thread_id)
.expect("spawned agent should receive an op");
let snapshot = manager
.get_thread(agent_id)
.await
.expect("spawned agent thread should exist")
.config_snapshot()
.await;
assert_eq!(snapshot.model, expected_model);
assert_eq!(snapshot.reasoning_effort, expected_reasoning_effort);
}
#[tokio::test]
async fn spawn_agent_returns_agent_id_without_task_name() {
let (mut session, turn) = make_session_and_context().await;
@@ -4,6 +4,7 @@ use crate::agent::control::render_input_preview;
use crate::agent::next_thread_spawn_depth;
use crate::agent::role::DEFAULT_ROLE_NAME;
use crate::agent::role::apply_role_to_config;
use crate::agent::role::default_fork_context_for_role;
use codex_protocol::AgentPath;
use codex_protocol::protocol::InterAgentCommunication;
use codex_protocol::protocol::Op;
@@ -64,14 +65,19 @@ impl ToolHandler for Handler {
.await;
let mut config =
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
apply_requested_spawn_agent_model_overrides(
&session,
turn.as_ref(),
&mut config,
args.model.as_deref(),
args.reasoning_effort,
)
.await?;
let fork_context = args
.fork_context
.unwrap_or_else(|| default_fork_context_for_role(&turn.config, role_name));
if !fork_context {
apply_requested_spawn_agent_model_overrides(
&session,
turn.as_ref(),
&mut config,
args.model.as_deref(),
args.reasoning_effort,
)
.await?;
}
apply_role_to_config(&mut config, role_name)
.await
.map_err(FunctionCallError::RespondToModel)?;
@@ -112,7 +118,7 @@ impl ToolHandler for Handler {
},
Some(spawn_source),
SpawnAgentOptions {
fork_parent_spawn_call_id: args.fork_context.then(|| call_id.clone()),
fork_parent_spawn_call_id: fork_context.then(|| call_id.clone()),
},
)
.await
@@ -204,8 +210,7 @@ struct SpawnAgentArgs {
agent_type: Option<String>,
model: Option<String>,
reasoning_effort: Option<ReasoningEffort>,
#[serde(default)]
fork_context: bool,
fork_context: Option<bool>,
}
#[derive(Debug, Serialize)]
@@ -467,6 +467,7 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() ->
description: Some("Custom role".to_string()),
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
})
@@ -515,6 +516,7 @@ async fn spawn_agent_tool_description_mentions_role_locked_settings() -> Result<
description: Some("Custom role".to_string()),
config_file: Some(role_path),
nickname_candidates: None,
fork_context: None,
},
);
});