mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore(config) include_collaboration_mode_instructions (#22383)
## Summary Adds include_collaboration_mode_instructions, which is a config equivalent to include_permissions_instructions for collaboration modes. Desired for situations where we want to disable this instruction from entering the context ## Testing - [x] Added unit test
This commit is contained in:
committed by
GitHub
Unverified
parent
862b2122ee
commit
8123bddb16
@@ -162,6 +162,9 @@ pub struct ConfigToml {
|
||||
/// Whether to inject the `<apps_instructions>` developer block.
|
||||
pub include_apps_instructions: Option<bool>,
|
||||
|
||||
/// Whether to inject the `<collaboration_mode>` developer block.
|
||||
pub include_collaboration_mode_instructions: Option<bool>,
|
||||
|
||||
/// Whether to inject the `<environment_context>` user block.
|
||||
pub include_environment_context: Option<bool>,
|
||||
|
||||
|
||||
@@ -57,6 +57,7 @@ pub struct ConfigProfile {
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub include_permissions_instructions: Option<bool>,
|
||||
pub include_apps_instructions: Option<bool>,
|
||||
pub include_collaboration_mode_instructions: Option<bool>,
|
||||
pub include_environment_context: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
pub experimental_use_freeform_apply_patch: Option<bool>,
|
||||
|
||||
@@ -622,6 +622,9 @@
|
||||
"include_apps_instructions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_collaboration_mode_instructions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_environment_context": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -4358,6 +4361,10 @@
|
||||
"description": "Whether to inject the `<apps_instructions>` developer block.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_collaboration_mode_instructions": {
|
||||
"description": "Whether to inject the `<collaboration_mode>` developer block.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_environment_context": {
|
||||
"description": "Whether to inject the `<environment_context>` user block.",
|
||||
"type": "boolean"
|
||||
|
||||
@@ -7353,6 +7353,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
guardian_policy_config: None,
|
||||
include_permissions_instructions: true,
|
||||
include_apps_instructions: true,
|
||||
include_collaboration_mode_instructions: true,
|
||||
include_skill_instructions: true,
|
||||
include_environment_context: true,
|
||||
compact_prompt: None,
|
||||
@@ -7799,6 +7800,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
guardian_policy_config: None,
|
||||
include_permissions_instructions: true,
|
||||
include_apps_instructions: true,
|
||||
include_collaboration_mode_instructions: true,
|
||||
include_skill_instructions: true,
|
||||
include_environment_context: true,
|
||||
compact_prompt: None,
|
||||
@@ -7959,6 +7961,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
guardian_policy_config: None,
|
||||
include_permissions_instructions: true,
|
||||
include_apps_instructions: true,
|
||||
include_collaboration_mode_instructions: true,
|
||||
include_skill_instructions: true,
|
||||
include_environment_context: true,
|
||||
compact_prompt: None,
|
||||
@@ -8104,6 +8107,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
guardian_policy_config: None,
|
||||
include_permissions_instructions: true,
|
||||
include_apps_instructions: true,
|
||||
include_collaboration_mode_instructions: true,
|
||||
include_skill_instructions: true,
|
||||
include_environment_context: true,
|
||||
compact_prompt: None,
|
||||
@@ -9341,6 +9345,7 @@ async fn prompt_instruction_blocks_can_be_disabled_from_config_and_profiles() ->
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"include_permissions_instructions = false
|
||||
include_apps_instructions = false
|
||||
include_collaboration_mode_instructions = false
|
||||
include_environment_context = false
|
||||
profile = "chatty"
|
||||
|
||||
@@ -9349,6 +9354,7 @@ include_instructions = false
|
||||
|
||||
[profiles.chatty]
|
||||
include_permissions_instructions = true
|
||||
include_collaboration_mode_instructions = true
|
||||
include_environment_context = true
|
||||
"#,
|
||||
)?;
|
||||
@@ -9361,6 +9367,7 @@ include_environment_context = true
|
||||
|
||||
assert!(config.include_permissions_instructions);
|
||||
assert!(!config.include_apps_instructions);
|
||||
assert!(config.include_collaboration_mode_instructions);
|
||||
assert!(!config.include_skill_instructions);
|
||||
assert!(config.include_environment_context);
|
||||
Ok(())
|
||||
|
||||
@@ -465,6 +465,9 @@ pub struct Config {
|
||||
/// Whether to inject the `<apps_instructions>` developer block.
|
||||
pub include_apps_instructions: bool,
|
||||
|
||||
/// Whether to inject the `<collaboration_mode>` developer block.
|
||||
pub include_collaboration_mode_instructions: bool,
|
||||
|
||||
/// Whether to inject the `<skills_instructions>` developer block.
|
||||
pub include_skill_instructions: bool,
|
||||
|
||||
@@ -2816,6 +2819,10 @@ impl Config {
|
||||
.include_apps_instructions
|
||||
.or(cfg.include_apps_instructions)
|
||||
.unwrap_or(true);
|
||||
let include_collaboration_mode_instructions = config_profile
|
||||
.include_collaboration_mode_instructions
|
||||
.or(cfg.include_collaboration_mode_instructions)
|
||||
.unwrap_or(true);
|
||||
let include_skill_instructions = cfg
|
||||
.skills
|
||||
.as_ref()
|
||||
@@ -3031,6 +3038,7 @@ impl Config {
|
||||
commit_attribution,
|
||||
include_permissions_instructions,
|
||||
include_apps_instructions,
|
||||
include_collaboration_mode_instructions,
|
||||
include_skill_instructions,
|
||||
include_environment_context,
|
||||
// The config.toml omits "_mode" because it's a config file. However, "_mode"
|
||||
|
||||
@@ -73,6 +73,10 @@ fn build_collaboration_mode_update_item(
|
||||
previous: Option<&TurnContextItem>,
|
||||
next: &TurnContext,
|
||||
) -> Option<String> {
|
||||
if !next.config.include_collaboration_mode_instructions {
|
||||
return None;
|
||||
}
|
||||
|
||||
let prev = previous?;
|
||||
if prev.collaboration_mode.as_ref() != Some(&next.collaboration_mode) {
|
||||
// If the next mode has empty developer instructions, this returns None and we emit no
|
||||
|
||||
@@ -133,6 +133,8 @@ fn save_config_resolved_fields(
|
||||
lock_config.model_verbosity = config.model_verbosity;
|
||||
lock_config.include_permissions_instructions = Some(config.include_permissions_instructions);
|
||||
lock_config.include_apps_instructions = Some(config.include_apps_instructions);
|
||||
lock_config.include_collaboration_mode_instructions =
|
||||
Some(config.include_collaboration_mode_instructions);
|
||||
lock_config.include_environment_context = Some(config.include_environment_context);
|
||||
lock_config.background_terminal_max_timeout = Some(config.background_terminal_max_timeout);
|
||||
|
||||
|
||||
@@ -2617,8 +2617,9 @@ impl Session {
|
||||
developer_sections.push(memory_prompt);
|
||||
}
|
||||
// Add developer instructions from collaboration_mode if they exist and are non-empty
|
||||
if let Some(collab_instructions) =
|
||||
CollaborationModeInstructions::from_collaboration_mode(&collaboration_mode)
|
||||
if turn_context.config.include_collaboration_mode_instructions
|
||||
&& let Some(collab_instructions) =
|
||||
CollaborationModeInstructions::from_collaboration_mode(&collaboration_mode)
|
||||
{
|
||||
developer_sections.push(collab_instructions.render());
|
||||
}
|
||||
|
||||
@@ -210,6 +210,60 @@ async fn collaboration_instructions_added_on_user_turn() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn collaboration_instructions_omitted_when_disabled() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let req = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.include_collaboration_mode_instructions = false;
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
let collaboration_mode = collab_mode_with_instructions(Some("turn instructions"));
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
environments: None,
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
cwd: test.config.cwd.to_path_buf(),
|
||||
approval_policy: test.config.permissions.approval_policy.value(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: test.config.legacy_sandbox_policy(),
|
||||
permission_profile: None,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: None,
|
||||
summary: Some(
|
||||
test.config
|
||||
.model_reasoning_summary
|
||||
.unwrap_or(codex_protocol::config_types::ReasoningSummary::Auto),
|
||||
),
|
||||
service_tier: None,
|
||||
collaboration_mode: Some(collaboration_mode),
|
||||
final_output_json_schema: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let input = req.single_request().input();
|
||||
let dev_texts = developer_texts(&input);
|
||||
assert_eq!(
|
||||
count_messages_containing(&dev_texts, COLLABORATION_MODE_OPEN_TAG),
|
||||
0
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn override_then_next_turn_uses_updated_collaboration_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -191,6 +191,7 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
|
||||
guardian_policy_config: None,
|
||||
include_permissions_instructions: false,
|
||||
include_apps_instructions: false,
|
||||
include_collaboration_mode_instructions: false,
|
||||
include_skill_instructions: false,
|
||||
include_environment_context: false,
|
||||
compact_prompt: None,
|
||||
|
||||
Reference in New Issue
Block a user