Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/config/src/config_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ pub struct ExperimentalRequestUserInput {
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct UpdatePlanToolConfig {
#[serde(default = "default_true")]
#[serde(default)]
pub enabled: bool,
}

Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -5546,7 +5546,7 @@
"additionalProperties": false,
"properties": {
"enabled": {
"default": true,
"default": false,
"type": "boolean"
}
},
Expand Down
14 changes: 11 additions & 3 deletions codex-rs/core/src/agent/control/spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,11 @@ impl AgentControl {
let multi_agent_v2_usage_hint_texts_to_filter: Vec<String> =
if multi_agent_version == MultiAgentVersion::V2 {
let parent_config = parent_thread.session.get_config().await;
let parent_usage_hints =
resolve_usage_hints(&parent_config.multi_agent_v2, /*catalog*/ None);
let parent_usage_hints = resolve_usage_hints(
&parent_config.multi_agent_v2,
/*catalog*/ None,
!parent_config.update_plan_enabled,
);
[parent_usage_hints.root, parent_usage_hints.subagent]
.into_iter()
.flatten()
Expand Down Expand Up @@ -1041,7 +1044,12 @@ impl AgentControl {
.as_ref()
.map(|hints| hints.subagent.clone())
.unwrap_or_else(|| {
resolve_usage_hints(&config.multi_agent_v2, /*catalog*/ None).subagent
resolve_usage_hints(
&config.multi_agent_v2,
/*catalog*/ None,
!config.update_plan_enabled,
)
.subagent
})
{
let subagent_usage_hint_message = ContextualUserFragment::into(subagent_usage_hint);
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ async fn run_compact_task_inner_impl(
let turn_input_len = turn_input.len();
let prompt = Prompt {
input: turn_input,
base_instructions: sess.get_base_instructions().await,
base_instructions: sess.get_prompt_base_instructions().await,
..Default::default()
};
let attempt_result = drain_to_completed(
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/compact_remote_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(super) async fn run_remote_compact_attempt(
) -> CodexResult<RemoteCompactAttempt> {
let turn_context = &step_context.turn;
let mut history = sess.clone_history().await;
let base_instructions = sess.get_base_instructions().await;
let base_instructions = sess.get_prompt_base_instructions().await;
let (rewritten_outputs, estimated_deleted_tokens) =
trim_function_call_history_to_fit_context_window(
&mut history,
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/compact_remote_v2_attempt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(super) async fn run_remote_compact_v2_attempt(
) -> CodexResult<RemoteCompactV2Attempt> {
let turn_context = &step_context.turn;
let mut history = sess.clone_history().await;
let base_instructions = sess.get_base_instructions().await;
let base_instructions = sess.get_prompt_base_instructions().await;
let (rewritten_outputs, estimated_deleted_tokens) =
trim_function_call_history_to_fit_context_window(
&mut history,
Expand Down
35 changes: 22 additions & 13 deletions codex-rs/core/src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,8 +566,8 @@ async fn load_config_resolves_non_prefixed_mcp_tool_servers() -> std::io::Result
async fn load_config_resolves_update_plan_enabled() -> std::io::Result<()> {
let codex_home = tempdir()?;
for (config_toml, expected_enabled) in [
("", true),
("[tools.update_plan]", true),
("", false),
("[tools.update_plan]", false),
("[tools.update_plan]\nenabled = false", false),
("[tools.update_plan]\nenabled = true", true),
] {
Expand Down Expand Up @@ -11604,7 +11604,9 @@ max_concurrent_threads_per_session = 17
for wait_agent_enabled in [true, false] {
let mut config = config.clone();
config.wait_agent_enabled = wait_agent_enabled;
let usage_hints = resolve_usage_hints(&config, /*catalog*/ None);
let usage_hints = resolve_usage_hints(
&config, /*catalog*/ None, /*omit_update_plan_instructions*/ false,
);
for hint in [usage_hints.root, usage_hints.subagent] {
let hint = hint.expect("default usage hints should be present").body();
assert!(hint.contains(concurrency_guidance));
Expand All @@ -11621,47 +11623,49 @@ max_concurrent_threads_per_session = 17
root: Some(String::new()),
subagent: Some(String::new()),
}),
/*omit_update_plan_instructions*/ false,
);
assert!(usage_hints.root.is_none() && usage_hints.subagent.is_none());
}

#[test]
fn multi_agent_v2_model_override_exposure_preserves_configured_usage_hints() {
let config_toml = toml::from_str(
r#"[features.multi_agent_v2]
r###"[features.multi_agent_v2]
enabled = true
root_agent_usage_hint_text = "Root guidance."
subagent_usage_hint_text = "Subagent guidance."
root_agent_usage_hint_text = "## Plan tool\nRoot guidance."
subagent_usage_hint_text = "## `update_plan`\nSubagent guidance."
expose_spawn_agent_model_overrides = true
"#,
"###,
)
.expect("multi-agent v2 config should parse");

let config = resolve_multi_agent_v2_config(&config_toml);
assert!(config.expose_spawn_agent_model_overrides);
assert_eq!(
config.root_agent_usage_hint_text.as_deref(),
Some("Root guidance.")
Some("## Plan tool\nRoot guidance.")
);
assert_eq!(
config.subagent_usage_hint_text.as_deref(),
Some("Subagent guidance.")
Some("## `update_plan`\nSubagent guidance.")
);
let usage_hints = resolve_usage_hints(
&config,
Some(&MultiAgentRoleMessages {
root: Some("Catalog root base.".to_string()),
subagent: Some("Catalog subagent base.".to_string()),
}),
/*omit_update_plan_instructions*/ true,
);
assert_eq!(
(
usage_hints.root.map(|hint| hint.body()),
usage_hints.subagent.map(|hint| hint.body()),
),
(
Some("Root guidance.".to_string()),
Some("Subagent guidance.".to_string()),
Some("## Plan tool\nRoot guidance.".to_string()),
Some("## `update_plan`\nSubagent guidance.".to_string()),
)
);
}
Expand All @@ -11673,9 +11677,13 @@ fn multi_agent_v2_exposes_model_overrides_by_default() {

let mut config = resolve_multi_agent_v2_config(&config_toml);
assert!(config.expose_spawn_agent_model_overrides);
let usage_hints = resolve_usage_hints(&config, /*catalog*/ None);
let usage_hints = resolve_usage_hints(
&config, /*catalog*/ None, /*omit_update_plan_instructions*/ false,
);
config.expose_spawn_agent_model_overrides = false;
let usage_hints_without_model_overrides = resolve_usage_hints(&config, /*catalog*/ None);
let usage_hints_without_model_overrides = resolve_usage_hints(
&config, /*catalog*/ None, /*omit_update_plan_instructions*/ false,
);

for (hint, hint_without_model_overrides) in [
(usage_hints.root, usage_hints_without_model_overrides.root),
Expand Down Expand Up @@ -11790,6 +11798,7 @@ subagent_usage_hint_text = ""
root: Some("catalog root".to_string()),
subagent: Some("catalog subagent".to_string()),
}),
/*omit_update_plan_instructions*/ false,
);
assert_eq!(
(
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2641,7 +2641,7 @@ fn resolve_update_plan_enabled(config_toml: &ConfigToml) -> bool {
.tools
.as_ref()
.and_then(|tools| tools.update_plan.as_ref())
.is_none_or(|config| config.enabled)
.is_some_and(|config| config.enabled)
}

fn resolve_orchestrator_feature_enabled(
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ mod subagent_notification;
mod token_budget_context;
mod turn_aborted;
mod unsupported_media;
mod update_plan_instructions;
mod user_instructions;
mod user_shell_command;
pub(crate) mod world_state;
Expand Down Expand Up @@ -108,6 +109,7 @@ pub(crate) use token_budget_context::TokenBudgetRemainingContext;
pub(crate) use token_budget_context::TokenBudgetReminder;
pub(crate) use turn_aborted::TurnAborted;
pub(crate) use unsupported_media::UnsupportedMedia;
pub use update_plan_instructions::without_update_plan_instructions;
pub(crate) use user_instructions::UserInstructions;
pub(crate) use user_shell_command::UserShellCommand;
pub(crate) use world_state::ManagedDeveloperInstructions;
63 changes: 63 additions & 0 deletions codex-rs/core/src/context/update_plan_instructions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//! Omits checklist-tool guidance from Codex-owned prompts, before adding caller text.

/// Call only for Codex-owned prompt text; custom instructions must remain unchanged.
pub fn without_update_plan_instructions(instructions: &str) -> String {
let lines = instructions.split_inclusive('\n').collect::<Vec<_>>();
let mut rendered = String::with_capacity(instructions.len());
let mut index = 0;
while index < lines.len() {
let line = lines[index].trim_end();
if matches!(
line,
"## Planning"
| "## `update_plan`"
| "## Plan tool"
| "## Plan Mode vs update_plan tool"
) {
let end = (index + 1..lines.len())
.find(|&next| lines[next].starts_with("# ") || lines[next].starts_with("## "))
.unwrap_or(lines.len());
let is_checklist_section = line != "## Planning"
|| lines[index..end].iter().any(|line| {
line.starts_with("You have access to an `update_plan` tool")
|| line.starts_with("When `update_plan` is available, follow this section")
});
if is_checklist_section {
index = end;
continue;
}
}

if line == "Progress visibility:"
&& lines
.get(index + 1)
.is_some_and(|line| line.starts_with("If update_plan is available"))
{
index += 2;
if lines.get(index).is_some_and(|line| line.trim().is_empty()) {
index += 1;
}
continue;
}

if line.starts_with("- Use the plan tool ")
|| line.starts_with("- If you create a checklist or task list,")
{
index += 1;
while index < lines.len()
&& (lines[index].starts_with(' ') || lines[index].starts_with('\t'))
{
index += 1;
}
continue;
}

rendered.push_str(lines[index]);
index += 1;
}
rendered
}

#[cfg(test)]
#[path = "update_plan_instructions_tests.rs"]
mod tests;
31 changes: 31 additions & 0 deletions codex-rs/core/src/context/update_plan_instructions_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//! Checks that only the known checklist sections are omitted.

use super::without_update_plan_instructions;
use pretty_assertions::assert_eq;

#[test]
fn disabled_omits_checklist_sections_and_preserves_other_planning() {
let instructions = "Before.\n\n## Planning\nYou have access to an `update_plan` tool which tracks steps.\n\n### Examples\nKeep steps current.\n\n## Work\nImplement.\n\n## `update_plan`\nUpdate the checklist.\n\n# Next\n## Planning\nDiscuss architecture and inspect update_plan before editing.\n";
assert_eq!(
without_update_plan_instructions(instructions),
"Before.\n\n## Work\nImplement.\n\n# Next\n## Planning\nDiscuss architecture and inspect update_plan before editing.\n",
);
}

#[test]
fn disabled_omits_legacy_tool_and_plan_mode_cross_reference_sections() {
let instructions = "# Plan Mode\nKeep planning.\n\n## Plan Mode vs update_plan tool\nThe tools are separate.\n\n## Execution\nDo not edit.\n\n## Plan tool\nWhen using the planning tool:\n- Update steps.\n";
assert_eq!(
without_update_plan_instructions(instructions),
"# Plan Mode\nKeep planning.\n\n## Execution\nDo not edit.\n\n",
);
}

#[test]
fn disabled_omits_only_checklist_list_items_and_goal_guidance() {
let instructions = "Keep working.\n- Use the plan tool to explain the work\n - Skip simple tasks.\n - Keep steps current.\n- Explain discoveries.\n- If you create a checklist or task list, update its statuses.\n\nProgress visibility:\nIf update_plan is available, use it for complex work.\n\nCompletion:\nVerify the result.\n";
assert_eq!(
without_update_plan_instructions(instructions),
"Keep working.\n- Explain discoveries.\n\nCompletion:\nVerify the result.\n",
);
}
26 changes: 26 additions & 0 deletions codex-rs/core/src/context/world_state/collaboration_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use super::PreviousSectionState;
use super::WorldStateHash;
use super::WorldStateSection;
use crate::context::ContextualUserFragment;
use crate::context::without_update_plan_instructions;
use codex_models_manager::collaboration_mode_presets::builtin_collaboration_mode_presets;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::ModeKind;
use codex_protocol::models::ContentItemKind;
Expand All @@ -22,6 +24,8 @@ impl CollaborationModeState {
pub(crate) fn from_collaboration_mode(
collaboration_mode: &CollaborationMode,
catalog_messages: Option<&CollaborationModeMessages>,
update_plan_enabled: bool,
custom_model_catalog: bool,
) -> Self {
let catalog_instructions =
catalog_messages.and_then(|messages| match collaboration_mode.mode {
Expand All @@ -36,6 +40,28 @@ impl CollaborationModeState {
.clone()
.filter(|instructions| !instructions.is_empty())
});
let instructions = instructions.map(|instructions| {
if update_plan_enabled {
return instructions;
}
// Clients send built-in presets through the override field too. Match the
// entire preset, never a heading that could also occur in custom text.
let builtin = match catalog_instructions {
Some(_) => !custom_model_catalog,
None => builtin_collaboration_mode_presets().iter().any(|preset| {
preset
.developer_instructions
.as_ref()
.and_then(Option::as_ref)
== Some(&instructions)
}),
};
if builtin {
without_update_plan_instructions(&instructions)
} else {
instructions
}
});
// Keep an empty-state snapshot so removing instructions clears retained history only once.
let fragment = CollaborationModeInstructions {
instructions: instructions.clone().unwrap_or_default(),
Expand Down
12 changes: 12 additions & 0 deletions codex-rs/core/src/context/world_state/collaboration_mode_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ fn instruction_updates_are_applied_once_in_retained_history() {
world_state.add_section(CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Default, instructions),
/*catalog_messages*/ None,
/*update_plan_enabled*/ true,
/*custom_model_catalog*/ false,
));
let expected: ResponseItem = ContextualUserFragment::into(CollaborationModeInstructions {
instructions: instructions.unwrap_or_default().to_string(),
Expand Down Expand Up @@ -84,6 +86,8 @@ fn catalog_collaboration_messages_select_mode_variant() {
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(mode, Some("legacy instructions")),
Some(&messages),
/*update_plan_enabled*/ true,
/*custom_model_catalog*/ false,
);

assert_eq!(state.instructions.as_deref(), Some(expected));
Expand All @@ -99,6 +103,8 @@ fn empty_catalog_collaboration_message_suppresses_legacy_instructions() {
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Plan, Some("legacy plan instructions")),
Some(&messages),
/*update_plan_enabled*/ true,
/*custom_model_catalog*/ false,
);

assert_eq!(
Expand All @@ -119,6 +125,8 @@ fn missing_catalog_collaboration_message_uses_legacy_instructions() {
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Plan, Some("legacy plan instructions")),
Some(&messages),
/*update_plan_enabled*/ true,
/*custom_model_catalog*/ false,
);

assert_eq!(
Expand All @@ -141,6 +149,8 @@ fn legacy_collaboration_mode_snapshots_refresh_catalog_messages_once() {
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Default, Some("stale legacy instructions")),
Some(&messages),
/*update_plan_enabled*/ true,
/*custom_model_catalog*/ false,
);

assert_eq!(
Expand Down Expand Up @@ -176,5 +186,7 @@ fn collaboration_mode_state(mode: ModeKind, instructions: &str) -> Collaboration
CollaborationModeState::from_collaboration_mode(
&collaboration_mode(mode, Some(instructions)),
/*catalog_messages*/ None,
/*update_plan_enabled*/ true,
/*custom_model_catalog*/ false,
)
}
Loading
Loading