mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Clarify locked role settings in spawn prompt (#14283)
- tell agents when a role pins model or reasoning effort so they know those settings are not changeable - add prompt-builder coverage for the locked-setting notes
This commit is contained in:
committed by
Michael Bolin
Unverified
parent
52a3bde6cc
commit
c32c445f1c
@@ -180,17 +180,49 @@ pub(crate) mod spawn_tool_spec {
|
||||
}
|
||||
|
||||
format!(
|
||||
r#"Optional type name for the new agent. If omitted, `{DEFAULT_ROLE_NAME}` is used.
|
||||
Available roles:
|
||||
{}
|
||||
"#,
|
||||
"Optional type name for the new agent. If omitted, `{DEFAULT_ROLE_NAME}` is used.\nAvailable roles:\n{}",
|
||||
formatted_roles.join("\n"),
|
||||
)
|
||||
}
|
||||
|
||||
fn format_role(name: &str, declaration: &AgentRoleConfig) -> String {
|
||||
if let Some(description) = &declaration.description {
|
||||
format!("{name}: {{\n{description}\n}}")
|
||||
let locked_settings_note = declaration
|
||||
.config_file
|
||||
.as_ref()
|
||||
.and_then(|config_file| {
|
||||
built_in::config_file_contents(config_file)
|
||||
.map(str::to_owned)
|
||||
.or_else(|| std::fs::read_to_string(config_file).ok())
|
||||
})
|
||||
.and_then(|contents| toml::from_str::<TomlValue>(&contents).ok())
|
||||
.map(|role_toml| {
|
||||
let model = role_toml
|
||||
.get("model")
|
||||
.and_then(TomlValue::as_str);
|
||||
let reasoning_effort = role_toml
|
||||
.get("model_reasoning_effort")
|
||||
.and_then(TomlValue::as_str);
|
||||
|
||||
match (model, reasoning_effort) {
|
||||
(Some(model), Some(reasoning_effort)) => format!(
|
||||
"\n- This role's model is set to `{model}` and its reasoning effort is set to `{reasoning_effort}`. These settings cannot be changed."
|
||||
),
|
||||
(Some(model), None) => {
|
||||
format!(
|
||||
"\n- This role's model is set to `{model}` and cannot be changed."
|
||||
)
|
||||
}
|
||||
(None, Some(reasoning_effort)) => {
|
||||
format!(
|
||||
"\n- This role's reasoning effort is set to `{reasoning_effort}` and cannot be changed."
|
||||
)
|
||||
}
|
||||
(None, None) => String::new(),
|
||||
}
|
||||
})
|
||||
.unwrap_or_default();
|
||||
format!("{name}: {{\n{description}{locked_settings_note}\n}}")
|
||||
} else {
|
||||
format!("{name}: no description")
|
||||
}
|
||||
@@ -901,6 +933,56 @@ enabled = false
|
||||
assert!(user_index < built_in_index);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_tool_spec_marks_role_locked_model_and_reasoning_effort() {
|
||||
let tempdir = TempDir::new().expect("create temp dir");
|
||||
let role_path = tempdir.path().join("researcher.toml");
|
||||
fs::write(
|
||||
&role_path,
|
||||
"developer_instructions = \"Research carefully\"\nmodel = \"gpt-5\"\nmodel_reasoning_effort = \"high\"\n",
|
||||
)
|
||||
.expect("write role config");
|
||||
let user_defined_roles = BTreeMap::from([(
|
||||
"researcher".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: Some("Research carefully.".to_string()),
|
||||
config_file: Some(role_path),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
)]);
|
||||
|
||||
let spec = spawn_tool_spec::build(&user_defined_roles);
|
||||
|
||||
assert!(spec.contains(
|
||||
"Research carefully.\n- This role's model is set to `gpt-5` and its reasoning effort is set to `high`. These settings cannot be changed."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_tool_spec_marks_role_locked_reasoning_effort_only() {
|
||||
let tempdir = TempDir::new().expect("create temp dir");
|
||||
let role_path = tempdir.path().join("reviewer.toml");
|
||||
fs::write(
|
||||
&role_path,
|
||||
"developer_instructions = \"Review carefully\"\nmodel_reasoning_effort = \"medium\"\n",
|
||||
)
|
||||
.expect("write role config");
|
||||
let user_defined_roles = BTreeMap::from([(
|
||||
"reviewer".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: Some("Review carefully.".to_string()),
|
||||
config_file: Some(role_path),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
)]);
|
||||
|
||||
let spec = spawn_tool_spec::build(&user_defined_roles);
|
||||
|
||||
assert!(spec.contains(
|
||||
"Review carefully.\n- This role's reasoning effort is set to `medium` and cannot be changed."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn built_in_config_file_contents_resolves_explorer_only() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -63,6 +63,44 @@ fn has_subagent_notification(req: &ResponsesRequest) -> bool {
|
||||
.any(|text| text.contains("<subagent_notification>"))
|
||||
}
|
||||
|
||||
fn tool_parameter_description(
|
||||
req: &ResponsesRequest,
|
||||
tool_name: &str,
|
||||
parameter_name: &str,
|
||||
) -> Option<String> {
|
||||
req.body_json()
|
||||
.get("tools")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
.and_then(|tools| {
|
||||
tools.iter().find_map(|tool| {
|
||||
if tool.get("name").and_then(serde_json::Value::as_str) == Some(tool_name) {
|
||||
tool.get("parameters")
|
||||
.and_then(|parameters| parameters.get("properties"))
|
||||
.and_then(|properties| properties.get(parameter_name))
|
||||
.and_then(|parameter| parameter.get("description"))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(str::to_owned)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn role_block(description: &str, role_name: &str) -> Option<String> {
|
||||
let role_header = format!("{role_name}: {{");
|
||||
let mut lines = description.lines().skip_while(|line| *line != role_header);
|
||||
let first_line = lines.next()?;
|
||||
let mut block = vec![first_line];
|
||||
for line in lines {
|
||||
if line.ends_with(": {") {
|
||||
break;
|
||||
}
|
||||
block.push(line);
|
||||
}
|
||||
Some(block.join("\n"))
|
||||
}
|
||||
|
||||
async fn wait_for_spawned_thread_id(test: &TestCodex) -> Result<String> {
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
loop {
|
||||
@@ -435,3 +473,58 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() ->
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawn_agent_tool_description_mentions_role_locked_settings() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let resp_mock = mount_sse_once_match(
|
||||
&server,
|
||||
|req: &wiremock::Request| body_contains(req, TURN_1_PROMPT),
|
||||
sse(vec![
|
||||
ev_response_created("resp-turn1-1"),
|
||||
ev_assistant_message("msg-turn1-1", "done"),
|
||||
ev_completed("resp-turn1-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::Collab)
|
||||
.expect("test config should allow feature update");
|
||||
let role_path = config.codex_home.join("custom-role.toml");
|
||||
std::fs::write(
|
||||
&role_path,
|
||||
format!(
|
||||
"developer_instructions = \"Stay focused\"\nmodel = \"{ROLE_MODEL}\"\nmodel_reasoning_effort = \"{ROLE_REASONING_EFFORT}\"\n",
|
||||
),
|
||||
)
|
||||
.expect("write role config");
|
||||
config.agent_roles.insert(
|
||||
"custom".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: Some("Custom role".to_string()),
|
||||
config_file: Some(role_path),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.submit_turn(TURN_1_PROMPT).await?;
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let agent_type_description = tool_parameter_description(&request, "spawn_agent", "agent_type")
|
||||
.expect("spawn_agent agent_type description");
|
||||
let custom_role_description =
|
||||
role_block(&agent_type_description, "custom").expect("custom role description");
|
||||
assert_eq!(
|
||||
custom_role_description,
|
||||
"custom: {\nCustom role\n- This role's model is set to `gpt-5.1-codex-max` and its reasoning effort is set to `high`. These settings cannot be changed.\n}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user