mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Support disabling tool suggest for specific tools. (#20072)
## Summary - Add `disable_tool_suggest` to app and plugin config, schema, and TypeScript output - Exclude disabled connectors and plugins from tool suggestion discovery - Persist "never show again" tool-suggestion choices back into `config.toml` - Update config docs and add coverage for connector and plugin suppression ## Testing - Added and updated unit tests for config persistence and tool-suggest filtering - Not run (not requested)
This commit is contained in:
committed by
GitHub
Unverified
parent
1211a90a35
commit
ebdf3a878c
@@ -46,6 +46,7 @@ use codex_config::types::NotificationMethod;
|
||||
use codex_config::types::Notifications;
|
||||
use codex_config::types::SandboxWorkspaceWrite;
|
||||
use codex_config::types::SkillsConfig;
|
||||
use codex_config::types::ToolSuggestDisabledTool;
|
||||
use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use codex_config::types::Tui;
|
||||
use codex_config::types::TuiKeymap;
|
||||
@@ -8144,6 +8145,7 @@ discoverables = [
|
||||
id: " ".to_string(),
|
||||
},
|
||||
],
|
||||
disabled_tools: Vec::new(),
|
||||
})
|
||||
);
|
||||
|
||||
@@ -8168,11 +8170,118 @@ discoverables = [
|
||||
id: "plugin_alpha@openai-curated".to_string(),
|
||||
},
|
||||
],
|
||||
disabled_tools: Vec::new(),
|
||||
}
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn tool_suggest_disabled_tools_load_from_config_toml() -> std::io::Result<()> {
|
||||
let cfg: ConfigToml = toml::from_str(
|
||||
r#"
|
||||
[tool_suggest]
|
||||
disabled_tools = [
|
||||
{ type = "connector", id = " connector_calendar " },
|
||||
{ type = "connector", id = "connector_calendar" },
|
||||
{ type = "connector", id = " " },
|
||||
{ type = "plugin", id = "slack@openai-curated" }
|
||||
]
|
||||
"#,
|
||||
)
|
||||
.expect("TOML deserialization should succeed");
|
||||
|
||||
assert_eq!(
|
||||
cfg.tool_suggest,
|
||||
Some(ToolSuggestConfig {
|
||||
discoverables: Vec::new(),
|
||||
disabled_tools: vec![
|
||||
ToolSuggestDisabledTool::connector(" connector_calendar "),
|
||||
ToolSuggestDisabledTool::connector("connector_calendar"),
|
||||
ToolSuggestDisabledTool::connector(" "),
|
||||
ToolSuggestDisabledTool::plugin("slack@openai-curated"),
|
||||
],
|
||||
})
|
||||
);
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides::default(),
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.tool_suggest,
|
||||
ToolSuggestConfig {
|
||||
discoverables: Vec::new(),
|
||||
disabled_tools: vec![
|
||||
ToolSuggestDisabledTool::connector("connector_calendar"),
|
||||
ToolSuggestDisabledTool::plugin("slack@openai-curated"),
|
||||
],
|
||||
}
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn tool_suggest_disabled_tools_merge_across_config_layers() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let workspace = TempDir::new()?;
|
||||
let workspace_key = workspace.path().to_string_lossy().replace('\\', "\\\\");
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
format!(
|
||||
r#"
|
||||
[projects."{workspace_key}"]
|
||||
trust_level = "trusted"
|
||||
|
||||
[tool_suggest]
|
||||
disabled_tools = [
|
||||
{{ type = "connector", id = " user_connector " }},
|
||||
{{ type = "plugin", id = "shared_plugin" }},
|
||||
{{ type = "connector", id = "project_connector" }},
|
||||
]
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
|
||||
let project_config_dir = workspace.path().join(".codex");
|
||||
std::fs::create_dir_all(&project_config_dir)?;
|
||||
std::fs::write(
|
||||
project_config_dir.join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[tool_suggest]
|
||||
disabled_tools = [
|
||||
{ type = "connector", id = "project_connector" },
|
||||
{ type = "plugin", id = "project_plugin" },
|
||||
{ type = "plugin", id = "shared_plugin" },
|
||||
]
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
cwd: Some(workspace.path().to_path_buf()),
|
||||
..Default::default()
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.tool_suggest.disabled_tools,
|
||||
vec![
|
||||
ToolSuggestDisabledTool::connector("user_connector"),
|
||||
ToolSuggestDisabledTool::plugin("shared_plugin"),
|
||||
ToolSuggestDisabledTool::connector("project_connector"),
|
||||
ToolSuggestDisabledTool::plugin("project_plugin"),
|
||||
]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn experimental_realtime_start_instructions_load_from_config_toml() -> std::io::Result<()> {
|
||||
let cfg: ConfigToml = toml::from_str(
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::path_utils::write_atomically;
|
||||
use anyhow::Context;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::ToolSuggestDisabledTool;
|
||||
use codex_features::FEATURES;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
@@ -10,6 +11,7 @@ use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tokio::task;
|
||||
@@ -57,6 +59,8 @@ pub enum ConfigEdit {
|
||||
RecordModelMigrationSeen { from: String, to: String },
|
||||
/// Replace the entire `[mcp_servers]` table.
|
||||
ReplaceMcpServers(BTreeMap<String, McpServerConfig>),
|
||||
/// Add a disabled tool suggestion under `[tool_suggest].disabled_tools`.
|
||||
AddToolSuggestDisabledTool(ToolSuggestDisabledTool),
|
||||
/// Set or clear a skill config entry under `[[skills.config]]` by path.
|
||||
SetSkillConfig { path: PathBuf, enabled: bool },
|
||||
/// Set or clear a skill config entry under `[[skills.config]]` by name.
|
||||
@@ -180,10 +184,13 @@ mod document_helpers {
|
||||
use codex_config::types::McpServerEnvVar;
|
||||
use codex_config::types::McpServerToolConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_config::types::ToolSuggestDisabledTool;
|
||||
use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use toml_edit::Array as TomlArray;
|
||||
use toml_edit::InlineTable;
|
||||
use toml_edit::Item as TomlItem;
|
||||
use toml_edit::Table as TomlTable;
|
||||
use toml_edit::Value as TomlValue;
|
||||
use toml_edit::value;
|
||||
|
||||
pub(super) fn ensure_table_for_write(item: &mut TomlItem) -> Option<&mut TomlTable> {
|
||||
@@ -379,6 +386,57 @@ mod document_helpers {
|
||||
table
|
||||
}
|
||||
|
||||
pub(super) fn parse_tool_suggest_disabled_tool(
|
||||
value: &TomlValue,
|
||||
) -> Option<ToolSuggestDisabledTool> {
|
||||
let table = value.as_inline_table()?;
|
||||
let kind = match table.get("type").and_then(TomlValue::as_str) {
|
||||
Some("connector") => ToolSuggestDiscoverableType::Connector,
|
||||
Some("plugin") => ToolSuggestDiscoverableType::Plugin,
|
||||
_ => return None,
|
||||
};
|
||||
let id = table.get("id").and_then(TomlValue::as_str)?;
|
||||
Some(ToolSuggestDisabledTool {
|
||||
kind,
|
||||
id: id.to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn parse_tool_suggest_disabled_tool_table(
|
||||
table: &TomlTable,
|
||||
) -> Option<ToolSuggestDisabledTool> {
|
||||
let kind = match table.get("type").and_then(TomlItem::as_str) {
|
||||
Some("connector") => ToolSuggestDiscoverableType::Connector,
|
||||
Some("plugin") => ToolSuggestDiscoverableType::Plugin,
|
||||
_ => return None,
|
||||
};
|
||||
let id = table.get("id").and_then(TomlItem::as_str)?;
|
||||
Some(ToolSuggestDisabledTool {
|
||||
kind,
|
||||
id: id.to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn tool_suggest_disabled_tools_value(
|
||||
disabled_tools: &[ToolSuggestDisabledTool],
|
||||
) -> TomlItem {
|
||||
let mut array = TomlArray::new();
|
||||
for disabled_tool in disabled_tools {
|
||||
let mut table = InlineTable::new();
|
||||
table.insert(
|
||||
"type",
|
||||
match disabled_tool.kind {
|
||||
ToolSuggestDiscoverableType::Connector => "connector",
|
||||
ToolSuggestDiscoverableType::Plugin => "plugin",
|
||||
}
|
||||
.into(),
|
||||
);
|
||||
table.insert("id", disabled_tool.id.clone().into());
|
||||
array.push(table);
|
||||
}
|
||||
TomlItem::Value(array.into())
|
||||
}
|
||||
|
||||
fn array_from_iter<I>(iter: I) -> TomlItem
|
||||
where
|
||||
I: Iterator<Item = String>,
|
||||
@@ -552,6 +610,9 @@ impl ConfigDocument {
|
||||
value(*acknowledged),
|
||||
)),
|
||||
ConfigEdit::ReplaceMcpServers(servers) => Ok(self.replace_mcp_servers(servers)),
|
||||
ConfigEdit::AddToolSuggestDisabledTool(disabled_tool) => {
|
||||
Ok(self.add_tool_suggest_disabled_tool(disabled_tool))
|
||||
}
|
||||
ConfigEdit::SetSkillConfig { path, enabled } => {
|
||||
Ok(self.set_skill_config(SkillConfigSelector::Path(path.clone()), *enabled))
|
||||
}
|
||||
@@ -590,6 +651,41 @@ impl ConfigDocument {
|
||||
self.remove(&resolved)
|
||||
}
|
||||
|
||||
fn add_tool_suggest_disabled_tool(&mut self, disabled_tool: &ToolSuggestDisabledTool) -> bool {
|
||||
let disabled_tools_item = self
|
||||
.doc
|
||||
.get("tool_suggest")
|
||||
.and_then(|item| item.as_table_like())
|
||||
.and_then(|table| table.get("disabled_tools"));
|
||||
let existing_from_array = disabled_tools_item
|
||||
.and_then(|item| item.as_value())
|
||||
.and_then(|value| value.as_array())
|
||||
.into_iter()
|
||||
.flat_map(|array| array.iter())
|
||||
.filter_map(document_helpers::parse_tool_suggest_disabled_tool);
|
||||
let existing_from_tables = disabled_tools_item
|
||||
.and_then(|item| match item {
|
||||
TomlItem::ArrayOfTables(array) => Some(array),
|
||||
_ => None,
|
||||
})
|
||||
.into_iter()
|
||||
.flat_map(|array| array.iter())
|
||||
.filter_map(document_helpers::parse_tool_suggest_disabled_tool_table);
|
||||
|
||||
let mut seen = HashSet::new();
|
||||
let disabled_tools = existing_from_array
|
||||
.chain(existing_from_tables)
|
||||
.chain(std::iter::once(disabled_tool.clone()))
|
||||
.filter_map(|disabled_tool| disabled_tool.normalized())
|
||||
.filter(|disabled_tool| seen.insert(disabled_tool.clone()))
|
||||
.collect::<Vec<_>>();
|
||||
self.write_value(
|
||||
Scope::Global,
|
||||
&["tool_suggest", "disabled_tools"],
|
||||
document_helpers::tool_suggest_disabled_tools_value(&disabled_tools),
|
||||
)
|
||||
}
|
||||
|
||||
fn clear_owned(&mut self, segments: &[String]) -> bool {
|
||||
self.remove(segments)
|
||||
}
|
||||
|
||||
@@ -46,6 +46,7 @@ use codex_config::types::OtelConfig;
|
||||
use codex_config::types::OtelConfigToml;
|
||||
use codex_config::types::OtelExporterKind;
|
||||
use codex_config::types::ToolSuggestConfig;
|
||||
use codex_config::types::ToolSuggestDisabledTool;
|
||||
use codex_config::types::ToolSuggestDiscoverable;
|
||||
use codex_config::types::TuiKeymap;
|
||||
use codex_config::types::TuiNotificationSettings;
|
||||
@@ -95,6 +96,7 @@ use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use serde::Deserialize;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -1416,10 +1418,29 @@ pub struct AgentRoleConfig {
|
||||
pub nickname_candidates: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
fn resolve_tool_suggest_config(config_toml: &ConfigToml) -> ToolSuggestConfig {
|
||||
let discoverables = config_toml
|
||||
.tool_suggest
|
||||
.as_ref()
|
||||
fn resolve_tool_suggest_config(
|
||||
config_toml: &ConfigToml,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> ToolSuggestConfig {
|
||||
resolve_tool_suggest_config_from_config(config_toml.tool_suggest.as_ref(), config_layer_stack)
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_tool_suggest_config_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> ToolSuggestConfig {
|
||||
let tool_suggest = config_layer_stack
|
||||
.effective_config()
|
||||
.get("tool_suggest")
|
||||
.cloned()
|
||||
.and_then(|value| value.try_into::<ToolSuggestConfig>().ok());
|
||||
resolve_tool_suggest_config_from_config(tool_suggest.as_ref(), config_layer_stack)
|
||||
}
|
||||
|
||||
fn resolve_tool_suggest_config_from_config(
|
||||
tool_suggest: Option<&ToolSuggestConfig>,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> ToolSuggestConfig {
|
||||
let discoverables = tool_suggest
|
||||
.into_iter()
|
||||
.flat_map(|tool_suggest| tool_suggest.discoverables.iter())
|
||||
.filter_map(|discoverable| {
|
||||
@@ -1434,8 +1455,47 @@ fn resolve_tool_suggest_config(config_toml: &ConfigToml) -> ToolSuggestConfig {
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let mut seen_disabled_tools = HashSet::new();
|
||||
let mut disabled_tools = Vec::new();
|
||||
let mut add_disabled_tool = |disabled_tool: ToolSuggestDisabledTool| {
|
||||
if let Some(disabled_tool) = disabled_tool.normalized()
|
||||
&& seen_disabled_tools.insert(disabled_tool.clone())
|
||||
{
|
||||
disabled_tools.push(disabled_tool);
|
||||
}
|
||||
};
|
||||
|
||||
ToolSuggestConfig { discoverables }
|
||||
let layers = config_layer_stack.get_layers(
|
||||
ConfigLayerStackOrdering::LowestPrecedenceFirst,
|
||||
/*include_disabled*/ false,
|
||||
);
|
||||
if layers.is_empty() {
|
||||
for disabled_tool in tool_suggest
|
||||
.into_iter()
|
||||
.flat_map(|tool_suggest| tool_suggest.disabled_tools.iter().cloned())
|
||||
{
|
||||
add_disabled_tool(disabled_tool);
|
||||
}
|
||||
} else {
|
||||
for layer in layers {
|
||||
let Some(tool_suggest) = layer
|
||||
.config
|
||||
.get("tool_suggest")
|
||||
.cloned()
|
||||
.and_then(|value| value.try_into::<ToolSuggestConfig>().ok())
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
for disabled_tool in tool_suggest.disabled_tools {
|
||||
add_disabled_tool(disabled_tool);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ToolSuggestConfig {
|
||||
discoverables,
|
||||
disabled_tools,
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_store_config(
|
||||
@@ -1840,7 +1900,7 @@ impl Config {
|
||||
.clone(),
|
||||
None => ConfigProfile::default(),
|
||||
};
|
||||
let tool_suggest = resolve_tool_suggest_config(&cfg);
|
||||
let tool_suggest = resolve_tool_suggest_config(&cfg, &config_layer_stack);
|
||||
let feature_overrides = FeatureOverrides {
|
||||
include_apply_patch_tool: include_apply_patch_tool_override,
|
||||
web_search_request: override_tools_web_search_request,
|
||||
|
||||
Reference in New Issue
Block a user