mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Clarify role-locked spawn settings
This commit is contained in:
+118
-25
@@ -123,16 +123,7 @@ pub(crate) async fn role_settings_locks(
|
||||
let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME);
|
||||
let role_layer_toml = load_role_layer_toml(config, role_name).await?;
|
||||
|
||||
Ok(RoleSettingsLocks {
|
||||
model: role_layer_toml
|
||||
.get("model")
|
||||
.and_then(TomlValue::as_str)
|
||||
.map(str::to_string),
|
||||
reasoning_effort: role_layer_toml
|
||||
.get("model_reasoning_effort")
|
||||
.and_then(TomlValue::as_str)
|
||||
.map(str::to_string),
|
||||
})
|
||||
Ok(role_settings_locks_from_layer_toml(&role_layer_toml))
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_role_config<'a>(
|
||||
@@ -199,6 +190,41 @@ async fn read_role_config_source<'a>(
|
||||
}
|
||||
}
|
||||
|
||||
fn role_settings_locks_from_layer_toml(role_layer_toml: &TomlValue) -> RoleSettingsLocks {
|
||||
let selected_profile = role_layer_toml
|
||||
.get("profile")
|
||||
.and_then(TomlValue::as_str)
|
||||
.and_then(|profile_name| {
|
||||
role_layer_toml
|
||||
.get("profiles")
|
||||
.and_then(TomlValue::as_table)
|
||||
.and_then(|profiles| profiles.get(profile_name))
|
||||
.and_then(TomlValue::as_table)
|
||||
});
|
||||
|
||||
RoleSettingsLocks {
|
||||
model: role_layer_toml
|
||||
.get("model")
|
||||
.and_then(TomlValue::as_str)
|
||||
.or_else(|| {
|
||||
selected_profile
|
||||
.and_then(|profile| profile.get("model").and_then(TomlValue::as_str))
|
||||
})
|
||||
.map(str::to_string),
|
||||
reasoning_effort: role_layer_toml
|
||||
.get("model_reasoning_effort")
|
||||
.and_then(TomlValue::as_str)
|
||||
.or_else(|| {
|
||||
selected_profile.and_then(|profile| {
|
||||
profile
|
||||
.get("model_reasoning_effort")
|
||||
.and_then(TomlValue::as_str)
|
||||
})
|
||||
})
|
||||
.map(str::to_string),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) mod spawn_tool_spec {
|
||||
use super::*;
|
||||
|
||||
@@ -295,31 +321,26 @@ Available roles:
|
||||
resolve_relative_paths_in_config_toml(role_config_toml, role_config_base)
|
||||
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
|
||||
|
||||
Ok(RoleSettingsLocks {
|
||||
model: role_layer_toml
|
||||
.get("model")
|
||||
.and_then(TomlValue::as_str)
|
||||
.map(str::to_string),
|
||||
reasoning_effort: role_layer_toml
|
||||
.get("model_reasoning_effort")
|
||||
.and_then(TomlValue::as_str)
|
||||
.map(str::to_string),
|
||||
})
|
||||
Ok(role_settings_locks_from_layer_toml(&role_layer_toml))
|
||||
}
|
||||
|
||||
fn render_lock_lines(locks: &RoleSettingsLocks) -> String {
|
||||
let mut lines = Vec::new();
|
||||
if let Some(model) = locks.model.as_deref() {
|
||||
lines.push(format!(
|
||||
"- This role sets model to `{model}`. This is set per this role and cannot be changed."
|
||||
"- `model` is set to `{model}` per this role and cannot be changed."
|
||||
));
|
||||
}
|
||||
if let Some(reasoning_effort) = locks.reasoning_effort.as_deref() {
|
||||
lines.push(format!(
|
||||
"- This role sets reasoning effort to `{reasoning_effort}`. This is set per this role and cannot be changed."
|
||||
"- `reasoning_effort` is set to `{reasoning_effort}` per this role and cannot be changed."
|
||||
));
|
||||
}
|
||||
lines.join("\n")
|
||||
if lines.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
format!("Locked settings:\n{}", lines.join("\n"))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1058,6 +1079,42 @@ enabled = false
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn role_settings_locks_reads_selected_profile_model_and_reasoning_effort() {
|
||||
let (home, mut config) = test_config_with_cli_overrides(Vec::new()).await;
|
||||
let role_path = write_role_config(
|
||||
&home,
|
||||
"profile-locked-role.toml",
|
||||
r#"profile = "planner"
|
||||
|
||||
[profiles.planner]
|
||||
model = "gpt-5.1-mini"
|
||||
model_reasoning_effort = "medium"
|
||||
"#,
|
||||
)
|
||||
.await;
|
||||
config.agent_roles.insert(
|
||||
"custom".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: None,
|
||||
config_file: Some(role_path),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
);
|
||||
|
||||
let locks = role_settings_locks(&config, Some("custom"))
|
||||
.await
|
||||
.expect("role locks should load");
|
||||
|
||||
assert_eq!(
|
||||
locks,
|
||||
RoleSettingsLocks {
|
||||
model: Some("gpt-5.1-mini".to_string()),
|
||||
reasoning_effort: Some("medium".to_string()),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_tool_spec_includes_lock_notices_for_user_defined_roles() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
@@ -1078,11 +1135,47 @@ enabled = false
|
||||
|
||||
let spec = spawn_tool_spec::build(&user_defined_roles);
|
||||
|
||||
assert!(spec.contains("Locked settings:"));
|
||||
assert!(
|
||||
spec.contains("- `model` is set to `gpt-5.1` per this role and cannot be changed.")
|
||||
);
|
||||
assert!(spec.contains(
|
||||
"This role sets model to `gpt-5.1`. This is set per this role and cannot be changed."
|
||||
"- `reasoning_effort` is set to `high` per this role and cannot be changed."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_tool_spec_includes_lock_notices_for_selected_profile_settings() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
let role_path = temp_dir.path().join("planner-role.toml");
|
||||
std::fs::write(
|
||||
&role_path,
|
||||
r#"profile = "planner"
|
||||
|
||||
[profiles.planner]
|
||||
model = "gpt-5.1-mini"
|
||||
model_reasoning_effort = "medium"
|
||||
"#,
|
||||
)
|
||||
.expect("write role config");
|
||||
let user_defined_roles = BTreeMap::from([(
|
||||
"planner".to_string(),
|
||||
AgentRoleConfig {
|
||||
description: Some("Plans work.".to_string()),
|
||||
config_file: Some(role_path),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
)]);
|
||||
|
||||
let spec = spawn_tool_spec::build(&user_defined_roles);
|
||||
|
||||
assert!(
|
||||
spec.contains(
|
||||
"- `model` is set to `gpt-5.1-mini` per this role and cannot be changed."
|
||||
)
|
||||
);
|
||||
assert!(spec.contains(
|
||||
"This role sets reasoning effort to `high`. This is set per this role and cannot be changed."
|
||||
"- `reasoning_effort` is set to `medium` per this role and cannot be changed."
|
||||
));
|
||||
}
|
||||
|
||||
|
||||
@@ -795,8 +795,7 @@ fn create_spawn_agent_tool(config: &ToolsConfig) -> ToolSpec {
|
||||
"model".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
"Optional model override for the new agent. Replaces the inherited model."
|
||||
.to_string(),
|
||||
"Optional model override for the new agent. Replaces the inherited model unless the selected role sets `model`, in which case the role setting wins.".to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
@@ -804,8 +803,7 @@ fn create_spawn_agent_tool(config: &ToolsConfig) -> ToolSpec {
|
||||
"reasoning_effort".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
"Optional reasoning effort override for the new agent. Replaces the inherited reasoning effort."
|
||||
.to_string(),
|
||||
"Optional reasoning effort override for the new agent. Replaces the inherited reasoning effort unless the selected role sets `reasoning_effort`, in which case the role setting wins.".to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
@@ -2729,6 +2727,43 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_agent_override_descriptions_note_role_locked_settings() {
|
||||
let config = test_config();
|
||||
let model_info =
|
||||
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &Features::with_defaults(),
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
});
|
||||
let tool = create_spawn_agent_tool(&tools_config);
|
||||
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else {
|
||||
panic!("spawn_agent should be a function tool");
|
||||
};
|
||||
let JsonSchema::Object { properties, .. } = parameters else {
|
||||
panic!("spawn_agent parameters should be an object");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
properties.get("model"),
|
||||
Some(&JsonSchema::String {
|
||||
description: Some(
|
||||
"Optional model override for the new agent. Replaces the inherited model unless the selected role sets `model`, in which case the role setting wins.".to_string(),
|
||||
),
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
properties.get("reasoning_effort"),
|
||||
Some(&JsonSchema::String {
|
||||
description: Some(
|
||||
"Optional reasoning effort override for the new agent. Replaces the inherited reasoning effort unless the selected role sets `reasoning_effort`, in which case the role setting wins.".to_string(),
|
||||
),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_permissions_requires_feature_flag() {
|
||||
let config = test_config();
|
||||
|
||||
@@ -842,12 +842,14 @@ async fn spawn_agent_tool_description_mentions_role_locked_model_and_reasoning()
|
||||
let request_body = req.single_request().body_json();
|
||||
let description = spawn_agent_type_description(&request_body)
|
||||
.expect("spawn_agent agent_type description should exist");
|
||||
assert!(description.contains(
|
||||
"This role sets model to `gpt-5.1`. This is set per this role and cannot be changed."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"This role sets reasoning effort to `high`. This is set per this role and cannot be changed."
|
||||
));
|
||||
assert!(description.contains("Locked settings:"));
|
||||
assert!(
|
||||
description.contains("- `model` is set to `gpt-5.1` per this role and cannot be changed.")
|
||||
);
|
||||
assert!(
|
||||
description
|
||||
.contains("- `reasoning_effort` is set to `high` per this role and cannot be changed.")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user