Enable plan tool by default (#5384)
## Summary - make the plan tool available by default by removing the feature flag and always registering the handler - drop plan-tool CLI and API toggles across the exec, TUI, MCP server, and app server code paths - update tests and configs to reflect the always-on plan tool and guard workspace restriction tests against env leakage ## Testing Manually tested the extension. ------ https://chatgpt.com/codex/tasks/task_i_68f67a3ff2d083209562a773f814c1f9
This commit is contained in:
@@ -1386,9 +1386,8 @@ async fn spawn_review_thread(
|
||||
let model = config.review_model.clone();
|
||||
let review_model_family = find_family_for_model(&model)
|
||||
.unwrap_or_else(|| parent_turn_context.client.get_model_family());
|
||||
// For reviews, disable plan, web_search, view_image regardless of global settings.
|
||||
// For reviews, disable web_search and view_image regardless of global settings.
|
||||
let mut review_features = config.features.clone();
|
||||
review_features.disable(crate::features::Feature::PlanTool);
|
||||
review_features.disable(crate::features::Feature::WebSearchRequest);
|
||||
review_features.disable(crate::features::Feature::ViewImageTool);
|
||||
review_features.disable(crate::features::Feature::StreamableShell);
|
||||
|
||||
@@ -216,9 +216,6 @@ pub struct Config {
|
||||
/// When set, restricts the login mechanism users may use.
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
|
||||
/// Include an experimental plan tool that the model can use to update its current plan and status of each step.
|
||||
pub include_plan_tool: bool,
|
||||
|
||||
/// Include the `apply_patch` tool for models that benefit from invoking
|
||||
/// file edits as a structured tool call. When unset, this falls back to the
|
||||
/// model family's default preference.
|
||||
@@ -1117,7 +1114,6 @@ pub struct ConfigOverrides {
|
||||
pub config_profile: Option<String>,
|
||||
pub codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
pub base_instructions: Option<String>,
|
||||
pub include_plan_tool: Option<bool>,
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub include_view_image_tool: Option<bool>,
|
||||
pub show_raw_agent_reasoning: Option<bool>,
|
||||
@@ -1147,7 +1143,6 @@ impl Config {
|
||||
config_profile: config_profile_key,
|
||||
codex_linux_sandbox_exe,
|
||||
base_instructions,
|
||||
include_plan_tool: include_plan_tool_override,
|
||||
include_apply_patch_tool: include_apply_patch_tool_override,
|
||||
include_view_image_tool: include_view_image_tool_override,
|
||||
show_raw_agent_reasoning,
|
||||
@@ -1174,7 +1169,6 @@ impl Config {
|
||||
};
|
||||
|
||||
let feature_overrides = FeatureOverrides {
|
||||
include_plan_tool: include_plan_tool_override,
|
||||
include_apply_patch_tool: include_apply_patch_tool_override,
|
||||
include_view_image_tool: include_view_image_tool_override,
|
||||
web_search_request: override_tools_web_search_request,
|
||||
@@ -1269,7 +1263,6 @@ impl Config {
|
||||
|
||||
let history = cfg.history.unwrap_or_default();
|
||||
|
||||
let include_plan_tool_flag = features.enabled(Feature::PlanTool);
|
||||
let include_apply_patch_tool_flag = features.enabled(Feature::ApplyPatchFreeform);
|
||||
let include_view_image_tool_flag = features.enabled(Feature::ViewImageTool);
|
||||
let tools_web_search_request = features.enabled(Feature::WebSearchRequest);
|
||||
@@ -1399,7 +1392,6 @@ impl Config {
|
||||
.unwrap_or("https://chatgpt.com/backend-api/".to_string()),
|
||||
forced_chatgpt_workspace_id,
|
||||
forced_login_method,
|
||||
include_plan_tool: include_plan_tool_flag,
|
||||
include_apply_patch_tool: include_apply_patch_tool_flag,
|
||||
tools_web_search_request,
|
||||
use_experimental_streamable_shell_tool,
|
||||
@@ -1765,7 +1757,6 @@ approve_all = true
|
||||
profiles.insert(
|
||||
"work".to_string(),
|
||||
ConfigProfile {
|
||||
include_plan_tool: Some(true),
|
||||
include_view_image_tool: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
@@ -1782,9 +1773,7 @@ approve_all = true
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
assert!(config.features.enabled(Feature::PlanTool));
|
||||
assert!(!config.features.enabled(Feature::ViewImageTool));
|
||||
assert!(config.include_plan_tool);
|
||||
assert!(!config.include_view_image_tool);
|
||||
|
||||
Ok(())
|
||||
@@ -1794,7 +1783,6 @@ approve_all = true
|
||||
fn feature_table_overrides_legacy_flags() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut entries = BTreeMap::new();
|
||||
entries.insert("plan_tool".to_string(), false);
|
||||
entries.insert("apply_patch_freeform".to_string(), false);
|
||||
let cfg = ConfigToml {
|
||||
features: Some(crate::features::FeaturesToml { entries }),
|
||||
@@ -1807,9 +1795,7 @@ approve_all = true
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
assert!(!config.features.enabled(Feature::PlanTool));
|
||||
assert!(!config.features.enabled(Feature::ApplyPatchFreeform));
|
||||
assert!(!config.include_plan_tool);
|
||||
assert!(!config.include_apply_patch_tool);
|
||||
|
||||
Ok(())
|
||||
@@ -2908,7 +2894,6 @@ model_verbosity = "high"
|
||||
base_instructions: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_plan_tool: false,
|
||||
include_apply_patch_tool: false,
|
||||
tools_web_search_request: false,
|
||||
use_experimental_streamable_shell_tool: false,
|
||||
@@ -2977,7 +2962,6 @@ model_verbosity = "high"
|
||||
base_instructions: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_plan_tool: false,
|
||||
include_apply_patch_tool: false,
|
||||
tools_web_search_request: false,
|
||||
use_experimental_streamable_shell_tool: false,
|
||||
@@ -3061,7 +3045,6 @@ model_verbosity = "high"
|
||||
base_instructions: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_plan_tool: false,
|
||||
include_apply_patch_tool: false,
|
||||
tools_web_search_request: false,
|
||||
use_experimental_streamable_shell_tool: false,
|
||||
@@ -3131,7 +3114,6 @@ model_verbosity = "high"
|
||||
base_instructions: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_plan_tool: false,
|
||||
include_apply_patch_tool: false,
|
||||
tools_web_search_request: false,
|
||||
use_experimental_streamable_shell_tool: false,
|
||||
|
||||
@@ -20,7 +20,6 @@ pub struct ConfigProfile {
|
||||
pub model_verbosity: Option<Verbosity>,
|
||||
pub chatgpt_base_url: Option<String>,
|
||||
pub experimental_instructions_file: Option<PathBuf>,
|
||||
pub include_plan_tool: Option<bool>,
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub include_view_image_tool: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
|
||||
@@ -33,8 +33,6 @@ pub enum Feature {
|
||||
StreamableShell,
|
||||
/// Use the official Rust MCP client (rmcp).
|
||||
RmcpClient,
|
||||
/// Include the plan tool.
|
||||
PlanTool,
|
||||
/// Include the freeform apply_patch tool.
|
||||
ApplyPatchFreeform,
|
||||
/// Include the view_image tool.
|
||||
@@ -74,7 +72,6 @@ pub struct Features {
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct FeatureOverrides {
|
||||
pub include_plan_tool: Option<bool>,
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub include_view_image_tool: Option<bool>,
|
||||
pub web_search_request: Option<bool>,
|
||||
@@ -83,7 +80,6 @@ pub struct FeatureOverrides {
|
||||
impl FeatureOverrides {
|
||||
fn apply(self, features: &mut Features) {
|
||||
LegacyFeatureToggles {
|
||||
include_plan_tool: self.include_plan_tool,
|
||||
include_apply_patch_tool: self.include_apply_patch_tool,
|
||||
include_view_image_tool: self.include_view_image_tool,
|
||||
tools_web_search: self.web_search_request,
|
||||
@@ -158,7 +154,6 @@ impl Features {
|
||||
}
|
||||
|
||||
let profile_legacy = LegacyFeatureToggles {
|
||||
include_plan_tool: config_profile.include_plan_tool,
|
||||
include_apply_patch_tool: config_profile.include_apply_patch_tool,
|
||||
include_view_image_tool: config_profile.include_view_image_tool,
|
||||
experimental_use_freeform_apply_patch: config_profile
|
||||
@@ -225,12 +220,6 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::PlanTool,
|
||||
key: "plan_tool",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ApplyPatchFreeform,
|
||||
key: "apply_patch_freeform",
|
||||
|
||||
@@ -29,10 +29,6 @@ const ALIASES: &[Alias] = &[
|
||||
legacy_key: "include_apply_patch_tool",
|
||||
feature: Feature::ApplyPatchFreeform,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "include_plan_tool",
|
||||
feature: Feature::PlanTool,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "include_view_image_tool",
|
||||
feature: Feature::ViewImageTool,
|
||||
@@ -55,7 +51,6 @@ pub(crate) fn feature_for_key(key: &str) -> Option<Feature> {
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub struct LegacyFeatureToggles {
|
||||
pub include_plan_tool: Option<bool>,
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub include_view_image_tool: Option<bool>,
|
||||
pub experimental_use_freeform_apply_patch: Option<bool>,
|
||||
@@ -68,12 +63,6 @@ pub struct LegacyFeatureToggles {
|
||||
|
||||
impl LegacyFeatureToggles {
|
||||
pub fn apply(self, features: &mut Features) {
|
||||
set_if_some(
|
||||
features,
|
||||
Feature::PlanTool,
|
||||
self.include_plan_tool,
|
||||
"include_plan_tool",
|
||||
);
|
||||
set_if_some(
|
||||
features,
|
||||
Feature::ApplyPatchFreeform,
|
||||
|
||||
@@ -25,7 +25,6 @@ pub enum ConfigShellToolType {
|
||||
#[derive(Debug, Clone)]
|
||||
pub(crate) struct ToolsConfig {
|
||||
pub shell_type: ConfigShellToolType,
|
||||
pub plan_tool: bool,
|
||||
pub apply_patch_tool_type: Option<ApplyPatchToolType>,
|
||||
pub web_search_request: bool,
|
||||
pub include_view_image_tool: bool,
|
||||
@@ -46,7 +45,6 @@ impl ToolsConfig {
|
||||
} = params;
|
||||
let use_streamable_shell_tool = features.enabled(Feature::StreamableShell);
|
||||
let experimental_unified_exec_tool = features.enabled(Feature::UnifiedExec);
|
||||
let include_plan_tool = features.enabled(Feature::PlanTool);
|
||||
let include_apply_patch_tool = features.enabled(Feature::ApplyPatchFreeform);
|
||||
let include_web_search_request = features.enabled(Feature::WebSearchRequest);
|
||||
let include_view_image_tool = features.enabled(Feature::ViewImageTool);
|
||||
@@ -73,7 +71,6 @@ impl ToolsConfig {
|
||||
|
||||
Self {
|
||||
shell_type,
|
||||
plan_tool: include_plan_tool,
|
||||
apply_patch_tool_type,
|
||||
web_search_request: include_web_search_request,
|
||||
include_view_image_tool,
|
||||
@@ -870,10 +867,8 @@ pub(crate) fn build_specs(
|
||||
builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone());
|
||||
builder.register_handler("read_mcp_resource", mcp_resource_handler);
|
||||
|
||||
if config.plan_tool {
|
||||
builder.push_spec(PLAN_TOOL.clone());
|
||||
builder.register_handler("update_plan", plan_handler);
|
||||
}
|
||||
builder.push_spec(PLAN_TOOL.clone());
|
||||
builder.register_handler("update_plan", plan_handler);
|
||||
|
||||
if let Some(apply_patch_tool_type) = &config.apply_patch_tool_type {
|
||||
match apply_patch_tool_type {
|
||||
@@ -1006,7 +1001,6 @@ mod tests {
|
||||
let model_family = find_family_for_model("codex-mini-latest")
|
||||
.expect("codex-mini-latest should be a valid model family");
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::PlanTool);
|
||||
features.enable(Feature::WebSearchRequest);
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let config = ToolsConfig::new(&ToolsConfigParams {
|
||||
@@ -1033,7 +1027,6 @@ mod tests {
|
||||
fn test_build_specs_default_shell() {
|
||||
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::PlanTool);
|
||||
features.enable(Feature::WebSearchRequest);
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let config = ToolsConfig::new(&ToolsConfigParams {
|
||||
@@ -1162,6 +1155,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"web_search",
|
||||
"view_image",
|
||||
"test_server/do_something_cool",
|
||||
@@ -1281,6 +1275,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"view_image",
|
||||
"test_server/cool",
|
||||
"test_server/do",
|
||||
@@ -1332,6 +1327,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"view_image",
|
||||
@@ -1340,7 +1336,7 @@ mod tests {
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
tools[7].spec,
|
||||
tools[8].spec,
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "dash/search".to_string(),
|
||||
parameters: JsonSchema::Object {
|
||||
@@ -1400,6 +1396,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"view_image",
|
||||
@@ -1407,7 +1404,7 @@ mod tests {
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
tools[7].spec,
|
||||
tools[8].spec,
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "dash/paginate".to_string(),
|
||||
parameters: JsonSchema::Object {
|
||||
@@ -1466,6 +1463,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"view_image",
|
||||
@@ -1473,7 +1471,7 @@ mod tests {
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
tools[7].spec,
|
||||
tools[8].spec,
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "dash/tags".to_string(),
|
||||
parameters: JsonSchema::Object {
|
||||
@@ -1534,6 +1532,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"view_image",
|
||||
@@ -1541,7 +1540,7 @@ mod tests {
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
tools[7].spec,
|
||||
tools[8].spec,
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "dash/value".to_string(),
|
||||
parameters: JsonSchema::Object {
|
||||
@@ -1639,6 +1638,7 @@ mod tests {
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"apply_patch",
|
||||
"web_search",
|
||||
"view_image",
|
||||
@@ -1647,7 +1647,7 @@ mod tests {
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
tools[7].spec,
|
||||
tools[8].spec,
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "test_server/do_something_cool".to_string(),
|
||||
parameters: JsonSchema::Object {
|
||||
|
||||
Reference in New Issue
Block a user