mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Remove ToolSearch feature toggle (#23389)
## Summary - mark `ToolSearch` as removed and ignore stale config writes for its legacy key - make search tool exposure depend only on model capability, not a feature toggle - remove app-server enablement support and prune now-obsolete test coverage/setup ## Verification - `cargo test -p codex-features` - `cargo test -p codex-tools` - `cargo test -p codex-core search_tool_requires_model_capability` - `cargo test -p codex-app-server experimental_feature_enablement_set_` ## Notes - This keeps the legacy config key as a no-op for compatibility while removing the ability to toggle the behavior off cleanly. - No developer-facing docs update outside the touched app-server README was needed.
This commit is contained in:
committed by
GitHub
Unverified
parent
6b54ced108
commit
daa11820b0
@@ -189,7 +189,7 @@ Example with notification opt-out:
|
||||
- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with reasoning effort options, `additionalSpeedTiers`, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata.
|
||||
- `modelProvider/capabilities/read` — read provider-level capabilities for the currently configured model provider.
|
||||
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. Pass `threadId` when showing feature state for an existing loaded thread so `enabled` is computed from that thread's refreshed config, including project-local config for the thread's cwd; if omitted, the server uses its default config resolution context. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
|
||||
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for the currently supported feature keys (`apps`, `memories`, `plugins`, `tool_search`, `tool_suggest`, `tool_call_mcp_elicitation`). For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default.
|
||||
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for the currently supported feature keys (`apps`, `memories`, `plugins`, `tool_suggest`, `tool_call_mcp_elicitation`). For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default.
|
||||
- `environment/add` — experimental; add or replace a named remote environment by `environmentId` and `execServerUrl` for later selection by `thread/start` or `turn/start`; returns `{}` and does not change the default environment.
|
||||
- `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). Built-in presets do not select a model; the Plan preset selects medium reasoning effort. This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly.
|
||||
- `skills/list` — list skills for one or more `cwd` values (optional `forceReload`).
|
||||
|
||||
@@ -53,7 +53,6 @@ const SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT: &[&str] = &[
|
||||
"mentions_v2",
|
||||
"plugins",
|
||||
"remote_control",
|
||||
"tool_search",
|
||||
"tool_suggest",
|
||||
"tool_call_mcp_elicitation",
|
||||
];
|
||||
|
||||
@@ -340,7 +340,6 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
|
||||
BTreeMap::from([
|
||||
("memories".to_string(), true),
|
||||
("plugins".to_string(), true),
|
||||
("tool_search".to_string(), true),
|
||||
("tool_suggest".to_string(), true),
|
||||
("tool_call_mcp_elicitation".to_string(), false),
|
||||
]),
|
||||
@@ -353,7 +352,6 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
|
||||
enablement: BTreeMap::from([
|
||||
("memories".to_string(), true),
|
||||
("plugins".to_string(), true),
|
||||
("tool_search".to_string(), true),
|
||||
("tool_suggest".to_string(), true),
|
||||
("tool_call_mcp_elicitation".to_string(), false),
|
||||
]),
|
||||
@@ -383,13 +381,6 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
|
||||
.and_then(|features| features.get("plugins")),
|
||||
Some(&json!(true))
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.additional
|
||||
.get("features")
|
||||
.and_then(|features| features.get("tool_search")),
|
||||
Some(&json!(true))
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.additional
|
||||
@@ -483,7 +474,7 @@ async fn experimental_feature_enablement_set_rejects_non_allowlisted_feature() -
|
||||
);
|
||||
assert!(
|
||||
error.message.contains(
|
||||
"apps, memories, mentions_v2, plugins, remote_control, tool_search, tool_suggest, tool_call_mcp_elicitation"
|
||||
"apps, memories, mentions_v2, plugins, remote_control, tool_suggest, tool_call_mcp_elicitation"
|
||||
),
|
||||
"{}",
|
||||
error.message
|
||||
|
||||
@@ -793,7 +793,6 @@ async fn request_plugin_install_requires_apps_and_plugins_features() {
|
||||
|
||||
for disabled_feature in [Feature::Apps, Feature::Plugins] {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::ToolSuggest);
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
@@ -832,7 +831,6 @@ async fn search_tool_is_hidden_without_deferred_tools() {
|
||||
let model_info = search_capable_model_info().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -863,7 +861,6 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
|
||||
let model_info = search_capable_model_info().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
|
||||
@@ -1653,7 +1653,6 @@ fn search_tool_description_lists_each_mcp_source_once() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1726,7 +1725,7 @@ fn search_tool_description_lists_each_mcp_source_once() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
fn search_tool_requires_model_capability() {
|
||||
let model_info = search_capable_model_info();
|
||||
let deferred_mcp_tools = Some(vec![deferred_mcp_tool(
|
||||
"_create_event",
|
||||
@@ -1759,26 +1758,6 @@ fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
);
|
||||
assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
|
||||
let mut features_without_tool_search = Features::with_defaults();
|
||||
features_without_tool_search.disable(Feature::ToolSearch);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features_without_tool_search,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
deferred_mcp_tools.clone(),
|
||||
&[],
|
||||
);
|
||||
assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
@@ -1801,8 +1780,7 @@ fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
#[test]
|
||||
fn no_search_tool_when_namespaces_disabled() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let mut tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1836,8 +1814,7 @@ fn no_search_tool_when_namespaces_disabled() {
|
||||
#[test]
|
||||
fn search_tool_registers_for_deferred_dynamic_tools() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1896,8 +1873,7 @@ fn search_tool_registers_for_deferred_dynamic_tools() {
|
||||
#[test]
|
||||
fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_disabled() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let mut tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1946,7 +1922,6 @@ fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_dis
|
||||
fn request_plugin_install_is_not_registered_without_feature_flag() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
features.disable(Feature::ToolSuggest);
|
||||
@@ -2036,7 +2011,6 @@ fn request_plugin_install_description_lists_discoverable_tools() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::ToolSuggest);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
|
||||
@@ -400,10 +400,6 @@ if (!tool) {
|
||||
.features
|
||||
.enable(Feature::Apps)
|
||||
.expect("test config should allow feature update");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ToolSearch)
|
||||
.expect("test config should allow feature update");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodeMode)
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
use anyhow::Result;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem;
|
||||
@@ -113,14 +112,6 @@ fn tool_search_output_has_namespace_child(
|
||||
namespace_child_tool(&output, namespace, tool_name).is_some()
|
||||
}
|
||||
|
||||
fn configure_apps_without_tool_search(config: &mut Config, apps_base_url: &str) {
|
||||
configure_search_capable_apps(config, apps_base_url);
|
||||
config
|
||||
.features
|
||||
.disable(Feature::ToolSearch)
|
||||
.expect("test config should allow feature update");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn search_tool_enabled_by_default_adds_tool_search() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -225,54 +216,6 @@ async fn always_defer_feature_hides_small_app_tool_sets() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn tool_search_disabled_exposes_apps_tools_directly() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let apps_server = AppsTestServer::mount_searchable(&server).await?;
|
||||
let mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
configure_apps_without_tool_search(config, apps_server.chatgpt_base_url.as_str())
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.submit_turn_with_approval_and_permission_profile(
|
||||
"list tools",
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let body = mock.single_request().body_json();
|
||||
let tools = tool_names(&body);
|
||||
assert!(!tools.iter().any(|name| name == TOOL_SEARCH_TOOL_NAME));
|
||||
assert!(
|
||||
namespace_child_tool(
|
||||
&body,
|
||||
SEARCH_CALENDAR_NAMESPACE,
|
||||
SEARCH_CALENDAR_CREATE_TOOL
|
||||
)
|
||||
.is_some()
|
||||
);
|
||||
assert!(
|
||||
namespace_child_tool(&body, SEARCH_CALENDAR_NAMESPACE, SEARCH_CALENDAR_LIST_TOOL).is_some()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn search_tool_is_hidden_for_api_key_auth() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -132,7 +132,7 @@ pub enum Feature {
|
||||
EnableMcpApps,
|
||||
/// Use the new path for the host-owned apps MCP server.
|
||||
AppsMcpPathOverride,
|
||||
/// Enable the tool_search tool for apps.
|
||||
/// Removed compatibility flag retained as a no-op now that tool_search is always enabled.
|
||||
ToolSearch,
|
||||
/// Always defer MCP tools behind tool_search instead of exposing small sets directly.
|
||||
ToolSearchAlwaysDeferMcpTools,
|
||||
@@ -428,6 +428,9 @@ impl Features {
|
||||
"apply_patch_freeform" => {
|
||||
continue;
|
||||
}
|
||||
"tool_search" => {
|
||||
continue;
|
||||
}
|
||||
"image_detail_original" => {
|
||||
continue;
|
||||
}
|
||||
@@ -946,8 +949,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::ToolSearch,
|
||||
key: "tool_search",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
stage: Stage::Removed,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ToolSearchAlwaysDeferMcpTools,
|
||||
|
||||
@@ -188,9 +188,10 @@ fn network_proxy_is_experimental_and_disabled_by_default() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_search_is_stable_and_enabled_by_default() {
|
||||
assert_eq!(Feature::ToolSearch.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::ToolSearch.default_enabled(), true);
|
||||
fn tool_search_is_removed_and_disabled_by_default() {
|
||||
assert_eq!(Feature::ToolSearch.stage(), Stage::Removed);
|
||||
assert_eq!(Feature::ToolSearch.default_enabled(), false);
|
||||
assert_eq!(feature_for_key("tool_search"), Some(Feature::ToolSearch));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -603,7 +604,6 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
features.enable(Feature::NetworkProxy);
|
||||
features.disable(Feature::ToolSearch);
|
||||
|
||||
let mut features_toml = FeaturesToml {
|
||||
multi_agent_v2: Some(FeatureToml::Config(crate::MultiAgentV2ConfigToml {
|
||||
|
||||
@@ -182,8 +182,7 @@ impl ToolsConfig {
|
||||
let include_multi_agent_v2 = features.enabled(Feature::MultiAgentV2);
|
||||
let include_collab_tools = include_multi_agent_v2 || features.enabled(Feature::Collab);
|
||||
let include_agent_jobs = features.enabled(Feature::SpawnCsv);
|
||||
let include_search_tool =
|
||||
model_info.supports_search_tool && features.enabled(Feature::ToolSearch);
|
||||
let include_search_tool = model_info.supports_search_tool;
|
||||
let include_tool_suggest = features.enabled(Feature::ToolSuggest)
|
||||
&& features.enabled(Feature::Apps)
|
||||
&& features.enabled(Feature::Plugins);
|
||||
|
||||
Reference in New Issue
Block a user