mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Make the update_plan tool opt-in (#41744)
## What changed - Default `tools.update_plan.enabled` to `false`; users can explicitly enable it to expose `update_plan`. - Remove bundled `update_plan` guidance from model, collaboration-mode, multi-agent, compaction, prewarm, and goal-continuation prompts when the tool is disabled. - Preserve custom base instructions, model catalog instructions, collaboration policies, and user goal text even when they mention planning or `update_plan`. ## Testing - Cover default and explicitly enabled tool registration, prompt consistency across request paths, and preservation of custom instructions. GitOrigin-RevId: a53964e6e72f98e2557dd3090fdb7caccb956527
This commit is contained in:
@@ -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;
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
@@ -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",
|
||||
);
|
||||
}
|
||||
@@ -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;
|
||||
@@ -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 {
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
@@ -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));
|
||||
@@ -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!(
|
||||
@@ -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!(
|
||||
@@ -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!(
|
||||
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user