mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Merge remote-tracking branch 'origin/dev/friel/agent-tools-namespace' into repair/watchdog-prompt-layering
This commit is contained in:
@@ -72,6 +72,7 @@ use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use codex_tools::create_tools_json_for_responses_api;
|
||||
use eventsource_stream::Event;
|
||||
use eventsource_stream::EventStreamError;
|
||||
use futures::StreamExt;
|
||||
@@ -107,7 +108,6 @@ use crate::response_debug_context::extract_response_debug_context;
|
||||
use crate::response_debug_context::extract_response_debug_context_from_api_error;
|
||||
use crate::response_debug_context::telemetry_api_error_message;
|
||||
use crate::response_debug_context::telemetry_transport_error_message;
|
||||
use crate::tools::spec::create_tools_json_for_responses_api;
|
||||
use crate::util::FeedbackRequestTags;
|
||||
use crate::util::emit_feedback_auth_recovery_tags;
|
||||
use crate::util::emit_feedback_request_tags_with_auth_env;
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
use crate::client_common::tools::ToolSpec;
|
||||
use crate::config::types::Personality;
|
||||
use crate::error::Result;
|
||||
pub use codex_api::common::ResponseEvent;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_tools::ToolSpec;
|
||||
use futures::Stream;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value;
|
||||
@@ -157,107 +157,11 @@ fn strip_total_output_header(output: &str) -> Option<(&str, u32)> {
|
||||
}
|
||||
|
||||
pub(crate) mod tools {
|
||||
use codex_protocol::config_types::WebSearchContextSize;
|
||||
use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters;
|
||||
use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLocation;
|
||||
use codex_protocol::config_types::WebSearchUserLocationType;
|
||||
pub(crate) use codex_tools::FreeformTool;
|
||||
pub(crate) use codex_tools::FreeformToolFormat;
|
||||
use codex_tools::JsonSchema;
|
||||
pub(crate) use codex_tools::ResponsesApiTool;
|
||||
pub(crate) use codex_tools::ToolSearchOutputTool;
|
||||
use serde::Serialize;
|
||||
|
||||
/// When serialized as JSON, this produces a valid "Tool" in the OpenAI
|
||||
/// Responses API.
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
#[serde(tag = "type")]
|
||||
pub(crate) enum ToolSpec {
|
||||
#[serde(rename = "function")]
|
||||
Function(ResponsesApiTool),
|
||||
#[serde(rename = "tool_search")]
|
||||
ToolSearch {
|
||||
execution: String,
|
||||
description: String,
|
||||
parameters: JsonSchema,
|
||||
},
|
||||
#[serde(rename = "local_shell")]
|
||||
LocalShell {},
|
||||
#[serde(rename = "image_generation")]
|
||||
ImageGeneration { output_format: String },
|
||||
// TODO: Understand why we get an error on web_search although the API docs say it's supported.
|
||||
// https://platform.openai.com/docs/guides/tools-web-search?api-mode=responses#:~:text=%7B%20type%3A%20%22web_search%22%20%7D%2C
|
||||
// The `external_web_access` field determines whether the web search is over cached or live content.
|
||||
// https://platform.openai.com/docs/guides/tools-web-search#live-internet-access
|
||||
#[serde(rename = "web_search")]
|
||||
WebSearch {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
external_web_access: Option<bool>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
filters: Option<ResponsesApiWebSearchFilters>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
user_location: Option<ResponsesApiWebSearchUserLocation>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
search_context_size: Option<WebSearchContextSize>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
search_content_types: Option<Vec<String>>,
|
||||
},
|
||||
#[serde(rename = "custom")]
|
||||
Freeform(FreeformTool),
|
||||
}
|
||||
|
||||
impl ToolSpec {
|
||||
pub(crate) fn name(&self) -> &str {
|
||||
match self {
|
||||
ToolSpec::Function(tool) => tool.name.as_str(),
|
||||
ToolSpec::ToolSearch { .. } => "tool_search",
|
||||
ToolSpec::LocalShell {} => "local_shell",
|
||||
ToolSpec::ImageGeneration { .. } => "image_generation",
|
||||
ToolSpec::WebSearch { .. } => "web_search",
|
||||
ToolSpec::Freeform(tool) => tool.name.as_str(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub(crate) struct ResponsesApiWebSearchFilters {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) allowed_domains: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
impl From<ConfigWebSearchFilters> for ResponsesApiWebSearchFilters {
|
||||
fn from(filters: ConfigWebSearchFilters) -> Self {
|
||||
Self {
|
||||
allowed_domains: filters.allowed_domains,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub(crate) struct ResponsesApiWebSearchUserLocation {
|
||||
#[serde(rename = "type")]
|
||||
pub(crate) r#type: WebSearchUserLocationType,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) country: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) region: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) city: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) timezone: Option<String>,
|
||||
}
|
||||
|
||||
impl From<ConfigWebSearchUserLocation> for ResponsesApiWebSearchUserLocation {
|
||||
fn from(user_location: ConfigWebSearchUserLocation) -> Self {
|
||||
Self {
|
||||
r#type: user_location.r#type,
|
||||
country: user_location.country,
|
||||
region: user_location.region,
|
||||
city: user_location.city,
|
||||
timezone: user_location.timezone,
|
||||
}
|
||||
}
|
||||
}
|
||||
pub(crate) use codex_tools::ToolSpec;
|
||||
}
|
||||
|
||||
pub struct ResponseStream {
|
||||
|
||||
@@ -4392,10 +4392,6 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
|
||||
handlers::reload_user_config(&sess).await;
|
||||
false
|
||||
}
|
||||
Op::ListCustomPrompts => {
|
||||
handlers::list_custom_prompts(&sess, sub.id.clone()).await;
|
||||
false
|
||||
}
|
||||
Op::ListSkills { cwds, force_reload } => {
|
||||
handlers::list_skills(&sess, sub.id.clone(), cwds, force_reload).await;
|
||||
false
|
||||
@@ -4521,13 +4517,11 @@ mod handlers {
|
||||
use crate::tasks::UserShellCommandMode;
|
||||
use crate::tasks::UserShellCommandTask;
|
||||
use crate::tasks::execute_user_shell_command;
|
||||
use codex_protocol::custom_prompts::CustomPrompt;
|
||||
use codex_protocol::protocol::CodexErrorInfo;
|
||||
use codex_protocol::protocol::ErrorEvent;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use codex_protocol::protocol::ListCustomPromptsResponseEvent;
|
||||
use codex_protocol::protocol::ListSkillsResponseEvent;
|
||||
use codex_protocol::protocol::McpServerRefreshConfig;
|
||||
use codex_protocol::protocol::Op;
|
||||
@@ -4987,23 +4981,6 @@ mod handlers {
|
||||
sess.send_event_raw(event).await;
|
||||
}
|
||||
|
||||
pub async fn list_custom_prompts(sess: &Session, sub_id: String) {
|
||||
let custom_prompts: Vec<CustomPrompt> =
|
||||
if let Some(dir) = crate::custom_prompts::default_prompts_dir() {
|
||||
crate::custom_prompts::discover_prompts_in(&dir).await
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::ListCustomPromptsResponse(ListCustomPromptsResponseEvent {
|
||||
custom_prompts,
|
||||
}),
|
||||
};
|
||||
sess.send_event_raw(event).await;
|
||||
}
|
||||
|
||||
pub async fn list_skills(
|
||||
sess: &Session,
|
||||
sub_id: String,
|
||||
@@ -6925,7 +6902,6 @@ fn realtime_text_for_event(msg: &EventMsg) -> Option<String> {
|
||||
| EventMsg::TurnDiff(_)
|
||||
| EventMsg::GetHistoryEntryResponse(_)
|
||||
| EventMsg::McpListToolsResponse(_)
|
||||
| EventMsg::ListCustomPromptsResponse(_)
|
||||
| EventMsg::ListSkillsResponse(_)
|
||||
| EventMsg::SkillsUpdateAvailable
|
||||
| EventMsg::PlanUpdate(_)
|
||||
|
||||
@@ -1,149 +0,0 @@
|
||||
use codex_protocol::custom_prompts::CustomPrompt;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tokio::fs;
|
||||
|
||||
/// Return the default prompts directory: `$CODEX_HOME/prompts`.
|
||||
/// If `CODEX_HOME` cannot be resolved, returns `None`.
|
||||
pub fn default_prompts_dir() -> Option<PathBuf> {
|
||||
crate::config::find_codex_home()
|
||||
.ok()
|
||||
.map(|home| home.join("prompts"))
|
||||
}
|
||||
|
||||
/// Discover prompt files in the given directory, returning entries sorted by name.
|
||||
/// Non-files are ignored. If the directory does not exist or cannot be read, returns empty.
|
||||
pub async fn discover_prompts_in(dir: &Path) -> Vec<CustomPrompt> {
|
||||
discover_prompts_in_excluding(dir, &HashSet::new()).await
|
||||
}
|
||||
|
||||
/// Discover prompt files in the given directory, excluding any with names in `exclude`.
|
||||
/// Returns entries sorted by name. Non-files are ignored. Missing/unreadable dir yields empty.
|
||||
pub async fn discover_prompts_in_excluding(
|
||||
dir: &Path,
|
||||
exclude: &HashSet<String>,
|
||||
) -> Vec<CustomPrompt> {
|
||||
let mut out: Vec<CustomPrompt> = Vec::new();
|
||||
let mut entries = match fs::read_dir(dir).await {
|
||||
Ok(entries) => entries,
|
||||
Err(_) => return out,
|
||||
};
|
||||
|
||||
while let Ok(Some(entry)) = entries.next_entry().await {
|
||||
let path = entry.path();
|
||||
let is_file_like = fs::metadata(&path)
|
||||
.await
|
||||
.map(|m| m.is_file())
|
||||
.unwrap_or(false);
|
||||
if !is_file_like {
|
||||
continue;
|
||||
}
|
||||
// Only include Markdown files with a .md extension.
|
||||
let is_md = path
|
||||
.extension()
|
||||
.and_then(|s| s.to_str())
|
||||
.map(|ext| ext.eq_ignore_ascii_case("md"))
|
||||
.unwrap_or(false);
|
||||
if !is_md {
|
||||
continue;
|
||||
}
|
||||
let Some(name) = path
|
||||
.file_stem()
|
||||
.and_then(|s| s.to_str())
|
||||
.map(str::to_string)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
if exclude.contains(&name) {
|
||||
continue;
|
||||
}
|
||||
let content = match fs::read_to_string(&path).await {
|
||||
Ok(s) => s,
|
||||
Err(_) => continue,
|
||||
};
|
||||
let (description, argument_hint, body) = parse_frontmatter(&content);
|
||||
out.push(CustomPrompt {
|
||||
name,
|
||||
path,
|
||||
content: body,
|
||||
description,
|
||||
argument_hint,
|
||||
});
|
||||
}
|
||||
out.sort_by(|a, b| a.name.cmp(&b.name));
|
||||
out
|
||||
}
|
||||
|
||||
/// Parse optional YAML-like frontmatter at the beginning of `content`.
|
||||
/// Supported keys:
|
||||
/// - `description`: short description shown in the slash popup
|
||||
/// - `argument-hint` or `argument_hint`: brief hint string shown after the description
|
||||
/// Returns (description, argument_hint, body_without_frontmatter).
|
||||
fn parse_frontmatter(content: &str) -> (Option<String>, Option<String>, String) {
|
||||
let mut segments = content.split_inclusive('\n');
|
||||
let Some(first_segment) = segments.next() else {
|
||||
return (None, None, String::new());
|
||||
};
|
||||
let first_line = first_segment.trim_end_matches(['\r', '\n']);
|
||||
if first_line.trim() != "---" {
|
||||
return (None, None, content.to_string());
|
||||
}
|
||||
|
||||
let mut desc: Option<String> = None;
|
||||
let mut hint: Option<String> = None;
|
||||
let mut frontmatter_closed = false;
|
||||
let mut consumed = first_segment.len();
|
||||
|
||||
for segment in segments {
|
||||
let line = segment.trim_end_matches(['\r', '\n']);
|
||||
let trimmed = line.trim();
|
||||
|
||||
if trimmed == "---" {
|
||||
frontmatter_closed = true;
|
||||
consumed += segment.len();
|
||||
break;
|
||||
}
|
||||
|
||||
if trimmed.is_empty() || trimmed.starts_with('#') {
|
||||
consumed += segment.len();
|
||||
continue;
|
||||
}
|
||||
|
||||
if let Some((k, v)) = trimmed.split_once(':') {
|
||||
let key = k.trim().to_ascii_lowercase();
|
||||
let mut val = v.trim().to_string();
|
||||
if val.len() >= 2 {
|
||||
let bytes = val.as_bytes();
|
||||
let first = bytes[0];
|
||||
let last = bytes[bytes.len() - 1];
|
||||
if (first == b'\"' && last == b'\"') || (first == b'\'' && last == b'\'') {
|
||||
val = val[1..val.len().saturating_sub(1)].to_string();
|
||||
}
|
||||
}
|
||||
match key.as_str() {
|
||||
"description" => desc = Some(val),
|
||||
"argument-hint" | "argument_hint" => hint = Some(val),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
consumed += segment.len();
|
||||
}
|
||||
|
||||
if !frontmatter_closed {
|
||||
// Unterminated frontmatter: treat input as-is.
|
||||
return (None, None, content.to_string());
|
||||
}
|
||||
|
||||
let body = if consumed >= content.len() {
|
||||
String::new()
|
||||
} else {
|
||||
content[consumed..].to_string()
|
||||
};
|
||||
(desc, hint, body)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "custom_prompts_tests.rs"]
|
||||
mod tests;
|
||||
@@ -1,95 +0,0 @@
|
||||
use super::*;
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_when_dir_missing() {
|
||||
let tmp = tempdir().expect("create TempDir");
|
||||
let missing = tmp.path().join("nope");
|
||||
let found = discover_prompts_in(&missing).await;
|
||||
assert!(found.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn discovers_and_sorts_files() {
|
||||
let tmp = tempdir().expect("create TempDir");
|
||||
let dir = tmp.path();
|
||||
fs::write(dir.join("b.md"), b"b").unwrap();
|
||||
fs::write(dir.join("a.md"), b"a").unwrap();
|
||||
fs::create_dir(dir.join("subdir")).unwrap();
|
||||
let found = discover_prompts_in(dir).await;
|
||||
let names: Vec<String> = found.into_iter().map(|e| e.name).collect();
|
||||
assert_eq!(names, vec!["a", "b"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn excludes_builtins() {
|
||||
let tmp = tempdir().expect("create TempDir");
|
||||
let dir = tmp.path();
|
||||
fs::write(dir.join("init.md"), b"ignored").unwrap();
|
||||
fs::write(dir.join("foo.md"), b"ok").unwrap();
|
||||
let mut exclude = HashSet::new();
|
||||
exclude.insert("init".to_string());
|
||||
let found = discover_prompts_in_excluding(dir, &exclude).await;
|
||||
let names: Vec<String> = found.into_iter().map(|e| e.name).collect();
|
||||
assert_eq!(names, vec!["foo"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skips_non_utf8_files() {
|
||||
let tmp = tempdir().expect("create TempDir");
|
||||
let dir = tmp.path();
|
||||
// Valid UTF-8 file
|
||||
fs::write(dir.join("good.md"), b"hello").unwrap();
|
||||
// Invalid UTF-8 content in .md file (e.g., lone 0xFF byte)
|
||||
fs::write(dir.join("bad.md"), vec![0xFF, 0xFE, b'\n']).unwrap();
|
||||
let found = discover_prompts_in(dir).await;
|
||||
let names: Vec<String> = found.into_iter().map(|e| e.name).collect();
|
||||
assert_eq!(names, vec!["good"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn discovers_symlinked_md_files() {
|
||||
let tmp = tempdir().expect("create TempDir");
|
||||
let dir = tmp.path();
|
||||
|
||||
// Create a real file
|
||||
fs::write(dir.join("real.md"), b"real content").unwrap();
|
||||
|
||||
// Create a symlink to the real file
|
||||
std::os::unix::fs::symlink(dir.join("real.md"), dir.join("link.md")).unwrap();
|
||||
|
||||
let found = discover_prompts_in(dir).await;
|
||||
let names: Vec<String> = found.into_iter().map(|e| e.name).collect();
|
||||
|
||||
// Both real and link should be discovered, sorted alphabetically
|
||||
assert_eq!(names, vec!["link", "real"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn parses_frontmatter_and_strips_from_body() {
|
||||
let tmp = tempdir().expect("create TempDir");
|
||||
let dir = tmp.path();
|
||||
let file = dir.join("withmeta.md");
|
||||
let text = "---\nname: ignored\ndescription: \"Quick review command\"\nargument-hint: \"[file] [priority]\"\n---\nActual body with $1 and $ARGUMENTS";
|
||||
fs::write(&file, text).unwrap();
|
||||
|
||||
let found = discover_prompts_in(dir).await;
|
||||
assert_eq!(found.len(), 1);
|
||||
let p = &found[0];
|
||||
assert_eq!(p.name, "withmeta");
|
||||
assert_eq!(p.description.as_deref(), Some("Quick review command"));
|
||||
assert_eq!(p.argument_hint.as_deref(), Some("[file] [priority]"));
|
||||
// Body should not include the frontmatter delimiters.
|
||||
assert_eq!(p.content, "Actual body with $1 and $ARGUMENTS");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_frontmatter_preserves_body_newlines() {
|
||||
let content = "---\r\ndescription: \"Line endings\"\r\nargument_hint: \"[arg]\"\r\n---\r\nFirst line\r\nSecond line\r\n";
|
||||
let (desc, hint, body) = parse_frontmatter(content);
|
||||
assert_eq!(desc.as_deref(), Some("Line endings"));
|
||||
assert_eq!(hint.as_deref(), Some("[arg]"));
|
||||
assert_eq!(body, "First line\r\nSecond line\r\n");
|
||||
}
|
||||
@@ -30,7 +30,6 @@ pub mod config_loader;
|
||||
pub mod connectors;
|
||||
mod context_manager;
|
||||
mod contextual_user_message;
|
||||
pub mod custom_prompts;
|
||||
pub use codex_utils_path::env;
|
||||
mod environment_context;
|
||||
pub mod error;
|
||||
|
||||
@@ -94,7 +94,7 @@ const MCP_TOOL_NAME_DELIMITER: &str = "__";
|
||||
const MAX_TOOL_NAME_LENGTH: usize = 64;
|
||||
|
||||
/// Default timeout for initializing MCP server & initially listing tools.
|
||||
pub const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
pub const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Default timeout for individual tool calls.
|
||||
const DEFAULT_TOOL_TIMEOUT: Duration = Duration::from_secs(120);
|
||||
|
||||
@@ -612,7 +612,7 @@ fn mcp_init_error_display_includes_startup_timeout_hint() {
|
||||
let display = mcp_init_error_display(server_name, /*entry*/ None, &err);
|
||||
|
||||
assert_eq!(
|
||||
"MCP client for `slow` timed out after 10 seconds. Add or adjust `startup_timeout_sec` in your config.toml:\n[mcp_servers.slow]\nstartup_timeout_sec = XX",
|
||||
"MCP client for `slow` timed out after 30 seconds. Add or adjust `startup_timeout_sec` in your config.toml:\n[mcp_servers.slow]\nstartup_timeout_sec = XX",
|
||||
display
|
||||
);
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ use super::sync_openai_plugins_repo;
|
||||
use crate::AuthManager;
|
||||
use crate::SkillMetadata;
|
||||
use crate::auth::CodexAuth;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::Config;
|
||||
use crate::config::ConfigService;
|
||||
use crate::config::ConfigServiceError;
|
||||
@@ -1008,28 +1009,7 @@ impl PluginsManager {
|
||||
auth_manager: Arc<AuthManager>,
|
||||
) {
|
||||
if config.features.enabled(Feature::Plugins) {
|
||||
let mut configured_curated_plugin_ids =
|
||||
configured_plugins_from_stack(&config.config_layer_stack)
|
||||
.into_keys()
|
||||
.filter_map(|plugin_key| match PluginId::parse(&plugin_key) {
|
||||
Ok(plugin_id)
|
||||
if plugin_id.marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME =>
|
||||
{
|
||||
Some(plugin_id)
|
||||
}
|
||||
Ok(_) => None,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
plugin_key,
|
||||
error = %err,
|
||||
"ignoring invalid configured plugin key during curated sync setup"
|
||||
);
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
configured_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key);
|
||||
self.start_curated_repo_sync(configured_curated_plugin_ids);
|
||||
self.start_curated_repo_sync();
|
||||
start_startup_remote_plugin_sync_once(
|
||||
Arc::clone(self),
|
||||
self.codex_home.clone(),
|
||||
@@ -1054,7 +1034,7 @@ impl PluginsManager {
|
||||
}
|
||||
}
|
||||
|
||||
fn start_curated_repo_sync(self: &Arc<Self>, configured_curated_plugin_ids: Vec<PluginId>) {
|
||||
fn start_curated_repo_sync(self: &Arc<Self>) {
|
||||
if CURATED_REPO_SYNC_STARTED.swap(true, Ordering::SeqCst) {
|
||||
return;
|
||||
}
|
||||
@@ -1065,6 +1045,8 @@ impl PluginsManager {
|
||||
.spawn(
|
||||
move || match sync_openai_plugins_repo(codex_home.as_path()) {
|
||||
Ok(curated_plugin_version) => {
|
||||
let configured_curated_plugin_ids =
|
||||
configured_curated_plugin_ids_from_codex_home(codex_home.as_path());
|
||||
match refresh_curated_plugin_cache(
|
||||
codex_home.as_path(),
|
||||
&curated_plugin_version,
|
||||
@@ -1333,7 +1315,13 @@ fn configured_plugins_from_stack(
|
||||
let Some(user_layer) = config_layer_stack.get_user_layer() else {
|
||||
return HashMap::new();
|
||||
};
|
||||
let Some(plugins_value) = user_layer.config.get("plugins") else {
|
||||
configured_plugins_from_user_config_value(&user_layer.config)
|
||||
}
|
||||
|
||||
fn configured_plugins_from_user_config_value(
|
||||
user_config: &toml::Value,
|
||||
) -> HashMap<String, PluginConfig> {
|
||||
let Some(plugins_value) = user_config.get("plugins") else {
|
||||
return HashMap::new();
|
||||
};
|
||||
match plugins_value.clone().try_into() {
|
||||
@@ -1345,6 +1333,60 @@ fn configured_plugins_from_stack(
|
||||
}
|
||||
}
|
||||
|
||||
fn configured_curated_plugin_ids(
|
||||
configured_plugins: HashMap<String, PluginConfig>,
|
||||
) -> Vec<PluginId> {
|
||||
let mut configured_curated_plugin_ids = configured_plugins
|
||||
.into_keys()
|
||||
.filter_map(|plugin_key| match PluginId::parse(&plugin_key) {
|
||||
Ok(plugin_id) if plugin_id.marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME => {
|
||||
Some(plugin_id)
|
||||
}
|
||||
Ok(_) => None,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
plugin_key,
|
||||
error = %err,
|
||||
"ignoring invalid configured plugin key during curated sync setup"
|
||||
);
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
configured_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key);
|
||||
configured_curated_plugin_ids
|
||||
}
|
||||
|
||||
fn configured_curated_plugin_ids_from_codex_home(codex_home: &Path) -> Vec<PluginId> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let user_config = match fs::read_to_string(&config_path) {
|
||||
Ok(user_config) => user_config,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Vec::new(),
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %config_path.display(),
|
||||
error = %err,
|
||||
"failed to read user config while refreshing curated plugin cache"
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
};
|
||||
|
||||
let user_config = match toml::from_str::<toml::Value>(&user_config) {
|
||||
Ok(user_config) => user_config,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %config_path.display(),
|
||||
error = %err,
|
||||
"failed to parse user config while refreshing curated plugin cache"
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
};
|
||||
|
||||
configured_curated_plugin_ids(configured_plugins_from_user_config_value(&user_config))
|
||||
}
|
||||
|
||||
fn load_plugin(
|
||||
config_name: String,
|
||||
plugin: &PluginConfig,
|
||||
|
||||
@@ -2207,6 +2207,43 @@ fn refresh_curated_plugin_cache_reinstalls_missing_configured_plugin_with_curren
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn configured_curated_plugin_ids_from_codex_home_reads_latest_user_config() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
|
||||
[plugins."slack@openai-curated"]
|
||||
enabled = true
|
||||
|
||||
[plugins."sample@debug"]
|
||||
enabled = true
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
configured_curated_plugin_ids_from_codex_home(tmp.path())
|
||||
.into_iter()
|
||||
.map(|plugin_id| plugin_id.as_key())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["slack@openai-curated".to_string()]
|
||||
);
|
||||
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
configured_curated_plugin_ids_from_codex_home(tmp.path()),
|
||||
Vec::<PluginId>::new()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_curated_plugin_cache_returns_false_when_configured_plugins_are_current() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -19,7 +19,6 @@ use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::tools::ToolRouter;
|
||||
use crate::tools::code_mode_description::augment_tool_spec_for_code_mode;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
use crate::tools::context::SharedTurnDiffTracker;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -29,6 +28,7 @@ use crate::tools::router::ToolCallSource;
|
||||
use crate::tools::router::ToolRouterParams;
|
||||
use crate::unified_exec::resolve_max_tokens;
|
||||
use codex_features::Feature;
|
||||
use codex_tools::tool_spec_to_code_mode_tool_definition;
|
||||
use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_output_truncation::formatted_truncate_text_content_items_with_policy;
|
||||
use codex_utils_output_truncation::truncate_function_output_items_with_policy;
|
||||
@@ -247,42 +247,13 @@ pub(super) async fn build_enabled_tools(
|
||||
let mut out = router
|
||||
.specs()
|
||||
.into_iter()
|
||||
.map(|spec| augment_tool_spec_for_code_mode(spec, /*code_mode_enabled*/ true))
|
||||
.filter_map(enabled_tool_from_spec)
|
||||
.filter_map(|spec| tool_spec_to_code_mode_tool_definition(&spec))
|
||||
.collect::<Vec<_>>();
|
||||
out.sort_by(|left, right| left.name.cmp(&right.name));
|
||||
out.dedup_by(|left, right| left.name == right.name);
|
||||
out
|
||||
}
|
||||
|
||||
fn enabled_tool_from_spec(spec: ToolSpec) -> Option<codex_code_mode::ToolDefinition> {
|
||||
let tool_name = spec.name().to_string();
|
||||
if !codex_code_mode::is_code_mode_nested_tool(&tool_name) {
|
||||
return None;
|
||||
}
|
||||
|
||||
match spec {
|
||||
ToolSpec::Function(tool) => Some(codex_code_mode::ToolDefinition {
|
||||
name: tool_name,
|
||||
description: tool.description,
|
||||
kind: codex_code_mode::CodeModeToolKind::Function,
|
||||
input_schema: serde_json::to_value(&tool.parameters).ok(),
|
||||
output_schema: tool.output_schema,
|
||||
}),
|
||||
ToolSpec::Freeform(tool) => Some(codex_code_mode::ToolDefinition {
|
||||
name: tool_name,
|
||||
description: tool.description,
|
||||
kind: codex_code_mode::CodeModeToolKind::Freeform,
|
||||
input_schema: None,
|
||||
output_schema: None,
|
||||
}),
|
||||
ToolSpec::LocalShell {}
|
||||
| ToolSpec::ImageGeneration { .. }
|
||||
| ToolSpec::ToolSearch { .. }
|
||||
| ToolSpec::WebSearch { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
async fn build_nested_router(exec: &ExecContext) -> ToolRouter {
|
||||
let nested_tools_config = exec.turn.tools_config.for_code_mode_nested_tools();
|
||||
let mcp_tools = exec
|
||||
|
||||
@@ -1,49 +0,0 @@
|
||||
use crate::client_common::tools::ToolSpec;
|
||||
|
||||
#[allow(unused_imports)]
|
||||
#[cfg(test)]
|
||||
pub(crate) use codex_code_mode::append_code_mode_sample;
|
||||
#[allow(unused_imports)]
|
||||
#[cfg(test)]
|
||||
pub(crate) use codex_code_mode::render_json_schema_to_typescript;
|
||||
|
||||
pub(crate) fn augment_tool_spec_for_code_mode(spec: ToolSpec, code_mode_enabled: bool) -> ToolSpec {
|
||||
if !code_mode_enabled {
|
||||
return spec;
|
||||
}
|
||||
|
||||
match spec {
|
||||
ToolSpec::Function(mut tool) => {
|
||||
let input_type = serde_json::to_value(&tool.parameters)
|
||||
.ok()
|
||||
.map(|schema| codex_code_mode::render_json_schema_to_typescript(&schema))
|
||||
.unwrap_or_else(|| "unknown".to_string());
|
||||
let output_type = tool
|
||||
.output_schema
|
||||
.as_ref()
|
||||
.map(codex_code_mode::render_json_schema_to_typescript)
|
||||
.unwrap_or_else(|| "unknown".to_string());
|
||||
tool.description = codex_code_mode::append_code_mode_sample(
|
||||
&tool.description,
|
||||
&tool.name,
|
||||
"args",
|
||||
input_type,
|
||||
output_type,
|
||||
);
|
||||
ToolSpec::Function(tool)
|
||||
}
|
||||
ToolSpec::Freeform(mut tool) => {
|
||||
if tool.name != codex_code_mode::PUBLIC_TOOL_NAME {
|
||||
tool.description = codex_code_mode::append_code_mode_sample(
|
||||
&tool.description,
|
||||
&tool.name,
|
||||
"input",
|
||||
"string".to_string(),
|
||||
"unknown".to_string(),
|
||||
);
|
||||
}
|
||||
ToolSpec::Freeform(tool)
|
||||
}
|
||||
other => other,
|
||||
}
|
||||
}
|
||||
@@ -1,104 +0,0 @@
|
||||
use super::append_code_mode_sample;
|
||||
use super::render_json_schema_to_typescript;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn render_json_schema_to_typescript_renders_object_properties() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"path": {"type": "string"},
|
||||
"recursive": {"type": "boolean"}
|
||||
},
|
||||
"required": ["path"],
|
||||
"additionalProperties": false
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
render_json_schema_to_typescript(&schema),
|
||||
"{ path: string; recursive?: boolean; }"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_json_schema_to_typescript_renders_anyof_unions() {
|
||||
let schema = json!({
|
||||
"anyOf": [
|
||||
{"const": "pending"},
|
||||
{"const": "done"},
|
||||
{"type": "number"}
|
||||
]
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
render_json_schema_to_typescript(&schema),
|
||||
"\"pending\" | \"done\" | number"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_json_schema_to_typescript_renders_additional_properties() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"tags": {
|
||||
"type": "array",
|
||||
"items": {"type": "string"}
|
||||
}
|
||||
},
|
||||
"additionalProperties": {"type": "integer"}
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
render_json_schema_to_typescript(&schema),
|
||||
"{ tags?: Array<string>; [key: string]: number; }"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_json_schema_to_typescript_sorts_object_properties() {
|
||||
let schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"structuredContent": {"type": "string"},
|
||||
"_meta": {"type": "string"},
|
||||
"isError": {"type": "boolean"},
|
||||
"content": {"type": "array", "items": {"type": "string"}}
|
||||
},
|
||||
"required": ["content"]
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
render_json_schema_to_typescript(&schema),
|
||||
"{ _meta?: string; content: Array<string>; isError?: boolean; structuredContent?: string; }"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_code_mode_sample_uses_global_tools_for_valid_identifiers() {
|
||||
assert_eq!(
|
||||
append_code_mode_sample(
|
||||
"desc",
|
||||
"mcp__ologs__get_profile",
|
||||
"args",
|
||||
"{ foo: string }".to_string(),
|
||||
"unknown".to_string(),
|
||||
),
|
||||
"desc\n\nexec tool declaration:\n```ts\ndeclare const tools: { mcp__ologs__get_profile(args: { foo: string }): Promise<unknown>; };\n```"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_code_mode_sample_normalizes_invalid_identifiers() {
|
||||
assert_eq!(
|
||||
append_code_mode_sample(
|
||||
"desc",
|
||||
"mcp__rmcp__echo-tool",
|
||||
"args",
|
||||
"{ foo: string }".to_string(),
|
||||
"unknown".to_string(),
|
||||
),
|
||||
"desc\n\nexec tool declaration:\n```ts\ndeclare const tools: { mcp__rmcp__echo_tool(args: { foo: string }): Promise<unknown>; };\n```"
|
||||
);
|
||||
}
|
||||
@@ -1,5 +1,4 @@
|
||||
pub mod code_mode;
|
||||
pub(crate) mod code_mode_description;
|
||||
pub mod context;
|
||||
pub(crate) mod discoverable;
|
||||
pub mod events;
|
||||
|
||||
@@ -24,6 +24,7 @@ use codex_hooks::HookToolInput;
|
||||
use codex_hooks::HookToolInputLocalShell;
|
||||
use codex_hooks::HookToolKind;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_tools::ConfiguredToolSpec;
|
||||
use codex_utils_readiness::Readiness;
|
||||
use serde_json::Value;
|
||||
use tracing::warn;
|
||||
@@ -436,21 +437,6 @@ impl ToolRegistry {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ConfiguredToolSpec {
|
||||
pub spec: ToolSpec,
|
||||
pub supports_parallel_tool_calls: bool,
|
||||
}
|
||||
|
||||
impl ConfiguredToolSpec {
|
||||
pub fn new(spec: ToolSpec, supports_parallel_tool_calls: bool) -> Self {
|
||||
Self {
|
||||
spec,
|
||||
supports_parallel_tool_calls,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub struct ToolRegistryBuilder {
|
||||
handlers: HashMap<String, Arc<dyn AnyToolHandler>>,
|
||||
specs: Vec<ConfiguredToolSpec>,
|
||||
|
||||
@@ -9,7 +9,6 @@ use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::discoverable::DiscoverableTool;
|
||||
use crate::tools::registry::AnyToolResult;
|
||||
use crate::tools::registry::ConfiguredToolSpec;
|
||||
use crate::tools::registry::ToolRegistry;
|
||||
use crate::tools::spec::ToolsConfig;
|
||||
use crate::tools::spec::build_specs_with_discoverable_tools;
|
||||
@@ -18,6 +17,7 @@ use codex_protocol::models::LocalShellAction;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::models::SearchToolCallParams;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use codex_tools::ConfiguredToolSpec;
|
||||
use rmcp::model::Tool;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
@@ -66,7 +66,7 @@ impl ToolRouter {
|
||||
specs
|
||||
.iter()
|
||||
.filter_map(|configured_tool| {
|
||||
if !codex_code_mode::is_code_mode_nested_tool(configured_tool.spec.name()) {
|
||||
if !codex_code_mode::is_code_mode_nested_tool(configured_tool.name()) {
|
||||
Some(configured_tool.spec.clone())
|
||||
} else {
|
||||
None
|
||||
@@ -101,7 +101,7 @@ impl ToolRouter {
|
||||
pub fn find_spec(&self, tool_name: &str) -> Option<ToolSpec> {
|
||||
self.specs
|
||||
.iter()
|
||||
.find(|config| config.spec.name() == tool_name)
|
||||
.find(|config| config.name() == tool_name)
|
||||
.map(|config| config.spec.clone())
|
||||
}
|
||||
|
||||
@@ -109,7 +109,7 @@ impl ToolRouter {
|
||||
self.specs
|
||||
.iter()
|
||||
.filter(|config| config.supports_parallel_tool_calls)
|
||||
.any(|config| config.spec.name() == tool_name)
|
||||
.any(|config| config.name() == tool_name)
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all, err)]
|
||||
|
||||
+132
-1547
File diff suppressed because it is too large
Load Diff
@@ -1,20 +1,42 @@
|
||||
use crate::client_common::tools::FreeformTool;
|
||||
use crate::config::test_config;
|
||||
use crate::models_manager::manager::ModelsManager;
|
||||
use crate::models_manager::model_info::with_config_overrides;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell::ShellType;
|
||||
use crate::tools::ToolRouter;
|
||||
use crate::tools::registry::ConfiguredToolSpec;
|
||||
use crate::tools::router::ToolRouterParams;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_protocol::models::VIEW_IMAGE_TOOL_NAME;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_tools::AdditionalProperties;
|
||||
use codex_tools::CommandToolOptions;
|
||||
use codex_tools::ConfiguredToolSpec;
|
||||
use codex_tools::FreeformTool;
|
||||
use codex_tools::ResponsesApiWebSearchFilters;
|
||||
use codex_tools::ResponsesApiWebSearchUserLocation;
|
||||
use codex_tools::SpawnAgentToolOptions;
|
||||
use codex_tools::ViewImageToolOptions;
|
||||
use codex_tools::WaitAgentTimeoutOptions;
|
||||
use codex_tools::create_close_agent_tool_v1;
|
||||
use codex_tools::create_close_agent_tool_v2;
|
||||
use codex_tools::create_exec_command_tool;
|
||||
use codex_tools::create_request_permissions_tool;
|
||||
use codex_tools::create_request_user_input_tool;
|
||||
use codex_tools::create_resume_agent_tool;
|
||||
use codex_tools::create_send_input_tool_v1;
|
||||
use codex_tools::create_send_message_tool;
|
||||
use codex_tools::create_spawn_agent_tool_v1;
|
||||
use codex_tools::create_spawn_agent_tool_v2;
|
||||
use codex_tools::create_view_image_tool;
|
||||
use codex_tools::create_wait_agent_tool_v1;
|
||||
use codex_tools::create_wait_agent_tool_v2;
|
||||
use codex_tools::create_write_stdin_tool;
|
||||
use codex_tools::mcp_tool_to_deferred_responses_api_tool;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use super::*;
|
||||
@@ -52,10 +74,6 @@ fn discoverable_connector(id: &str, name: &str, description: &str) -> Discoverab
|
||||
}))
|
||||
}
|
||||
|
||||
fn windows_shell_safety_description() -> String {
|
||||
format!("\n\n{}", super::windows_destructive_filesystem_guidance())
|
||||
}
|
||||
|
||||
fn search_capable_model_info() -> ModelInfo {
|
||||
let config = test_config();
|
||||
let mut model_info =
|
||||
@@ -105,23 +123,12 @@ fn deferred_responses_api_tool_serializes_with_defer_loading() {
|
||||
);
|
||||
}
|
||||
|
||||
fn tool_name(tool: &ToolSpec) -> &str {
|
||||
match tool {
|
||||
ToolSpec::Function(ResponsesApiTool { name, .. }) => name,
|
||||
ToolSpec::ToolSearch { .. } => "tool_search",
|
||||
ToolSpec::LocalShell {} => "local_shell",
|
||||
ToolSpec::ImageGeneration { .. } => "image_generation",
|
||||
ToolSpec::WebSearch { .. } => "web_search",
|
||||
ToolSpec::Freeform(FreeformTool { name, .. }) => name,
|
||||
}
|
||||
}
|
||||
|
||||
// Avoid order-based assertions; compare via set containment instead.
|
||||
fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[&str]) {
|
||||
use std::collections::HashSet;
|
||||
let mut names = HashSet::new();
|
||||
let mut duplicates = Vec::new();
|
||||
for name in tools.iter().map(|t| tool_name(&t.spec)) {
|
||||
for name in tools.iter().map(ConfiguredToolSpec::name) {
|
||||
if !names.insert(name) {
|
||||
duplicates.push(name);
|
||||
}
|
||||
@@ -141,7 +148,7 @@ fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[&
|
||||
fn assert_lacks_tool_name(tools: &[ConfiguredToolSpec], expected_absent: &str) {
|
||||
let names = tools
|
||||
.iter()
|
||||
.map(|tool| tool_name(&tool.spec))
|
||||
.map(ConfiguredToolSpec::name)
|
||||
.collect::<Vec<_>>();
|
||||
assert!(
|
||||
!names.contains(&expected_absent),
|
||||
@@ -159,10 +166,31 @@ fn shell_tool_name(config: &ToolsConfig) -> Option<&'static str> {
|
||||
}
|
||||
}
|
||||
|
||||
fn request_user_input_tool_spec(default_mode_request_user_input: bool) -> ToolSpec {
|
||||
create_request_user_input_tool(request_user_input_tool_description(
|
||||
default_mode_request_user_input,
|
||||
))
|
||||
}
|
||||
|
||||
fn spawn_agent_tool_options(config: &ToolsConfig) -> SpawnAgentToolOptions<'_> {
|
||||
SpawnAgentToolOptions {
|
||||
available_models: &config.available_models,
|
||||
agent_type_description: crate::agent::role::spawn_tool_spec::build(&config.agent_roles),
|
||||
}
|
||||
}
|
||||
|
||||
fn wait_agent_timeout_options() -> WaitAgentTimeoutOptions {
|
||||
WaitAgentTimeoutOptions {
|
||||
default_timeout_ms: DEFAULT_WAIT_TIMEOUT_MS,
|
||||
min_timeout_ms: MIN_WAIT_TIMEOUT_MS,
|
||||
max_timeout_ms: MAX_WAIT_TIMEOUT_MS,
|
||||
}
|
||||
}
|
||||
|
||||
fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a ConfiguredToolSpec {
|
||||
tools
|
||||
.iter()
|
||||
.find(|tool| tool_name(&tool.spec) == expected_name)
|
||||
.find(|tool| tool.name() == expected_name)
|
||||
.unwrap_or_else(|| panic!("expected tool {expected_name}"))
|
||||
}
|
||||
|
||||
@@ -198,7 +226,8 @@ fn strip_descriptions_tool(spec: &mut ToolSpec) {
|
||||
ToolSpec::Function(ResponsesApiTool { parameters, .. }) => {
|
||||
strip_descriptions_schema(parameters);
|
||||
}
|
||||
ToolSpec::Freeform(_)
|
||||
ToolSpec::Namespace(_)
|
||||
| ToolSpec::Freeform(_)
|
||||
| ToolSpec::LocalShell {}
|
||||
| ToolSpec::ImageGeneration { .. }
|
||||
| ToolSpec::WebSearch { .. } => {}
|
||||
@@ -294,7 +323,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() {
|
||||
let mut actual: BTreeMap<String, ToolSpec> = BTreeMap::from([]);
|
||||
let mut duplicate_names = Vec::new();
|
||||
for t in &tools {
|
||||
let name = tool_name(&t.spec).to_string();
|
||||
let name = t.name().to_string();
|
||||
if actual.insert(name.clone(), t.spec.clone()).is_some() {
|
||||
duplicate_names.push(name);
|
||||
}
|
||||
@@ -307,12 +336,13 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() {
|
||||
// Build expected from the same helpers used by the builder.
|
||||
let mut expected: BTreeMap<String, ToolSpec> = BTreeMap::from([]);
|
||||
for spec in [
|
||||
create_exec_command_tool(
|
||||
/*allow_login_shell*/ true, /*exec_permission_approvals_enabled*/ false,
|
||||
),
|
||||
create_exec_command_tool(CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
}),
|
||||
create_write_stdin_tool(),
|
||||
PLAN_TOOL.clone(),
|
||||
create_request_user_input_tool(CollaborationModesConfig::default()),
|
||||
request_user_input_tool_spec(/*default_mode_request_user_input*/ false),
|
||||
create_apply_patch_freeform_tool(),
|
||||
ToolSpec::WebSearch {
|
||||
external_web_access: Some(true),
|
||||
@@ -321,36 +351,38 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() {
|
||||
search_context_size: None,
|
||||
search_content_types: None,
|
||||
},
|
||||
create_view_image_tool(config.can_request_original_image_detail),
|
||||
create_view_image_tool(ViewImageToolOptions {
|
||||
can_request_original_image_detail: config.can_request_original_image_detail,
|
||||
}),
|
||||
] {
|
||||
expected.insert(tool_name(&spec).to_string(), spec);
|
||||
expected.insert(spec.name().to_string(), spec);
|
||||
}
|
||||
let collab_specs = if config.multi_agent_v2 {
|
||||
vec![
|
||||
create_spawn_agent_tool_v2(&config),
|
||||
create_spawn_agent_tool_v2(spawn_agent_tool_options(&config)),
|
||||
create_send_message_tool(),
|
||||
create_wait_agent_tool_v2(),
|
||||
create_wait_agent_tool_v2(wait_agent_timeout_options()),
|
||||
create_close_agent_tool_v2(),
|
||||
]
|
||||
} else {
|
||||
vec![
|
||||
create_spawn_agent_tool_v1(&config),
|
||||
create_spawn_agent_tool_v1(spawn_agent_tool_options(&config)),
|
||||
create_send_input_tool_v1(),
|
||||
create_wait_agent_tool_v1(),
|
||||
create_wait_agent_tool_v1(wait_agent_timeout_options()),
|
||||
create_close_agent_tool_v1(),
|
||||
]
|
||||
};
|
||||
for spec in collab_specs {
|
||||
expected.insert(tool_name(&spec).to_string(), spec);
|
||||
expected.insert(spec.name().to_string(), spec);
|
||||
}
|
||||
if !config.multi_agent_v2 {
|
||||
let spec = create_resume_agent_tool();
|
||||
expected.insert(tool_name(&spec).to_string(), spec);
|
||||
expected.insert(spec.name().to_string(), spec);
|
||||
}
|
||||
|
||||
if config.exec_permission_approvals_enabled {
|
||||
let spec = create_request_permissions_tool();
|
||||
expected.insert(tool_name(&spec).to_string(), spec);
|
||||
let spec = create_request_permissions_tool(request_permissions_tool_description());
|
||||
expected.insert(spec.name().to_string(), spec);
|
||||
}
|
||||
|
||||
// Exact name set match — this is the only test allowed to fail when tools change.
|
||||
@@ -737,7 +769,7 @@ fn request_user_input_description_reflects_default_mode_feature_flag() {
|
||||
let request_user_input_tool = find_tool(&tools, "request_user_input");
|
||||
assert_eq!(
|
||||
request_user_input_tool.spec,
|
||||
create_request_user_input_tool(CollaborationModesConfig::default())
|
||||
request_user_input_tool_spec(/*default_mode_request_user_input*/ false)
|
||||
);
|
||||
|
||||
features.enable(Feature::DefaultModeRequestUserInput);
|
||||
@@ -761,9 +793,7 @@ fn request_user_input_description_reflects_default_mode_feature_flag() {
|
||||
let request_user_input_tool = find_tool(&tools, "request_user_input");
|
||||
assert_eq!(
|
||||
request_user_input_tool.spec,
|
||||
create_request_user_input_tool(CollaborationModesConfig {
|
||||
default_mode_request_user_input: true,
|
||||
})
|
||||
request_user_input_tool_spec(/*default_mode_request_user_input*/ true)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -813,7 +843,7 @@ fn request_permissions_requires_feature_flag() {
|
||||
let request_permissions_tool = find_tool(&tools, "request_permissions");
|
||||
assert_eq!(
|
||||
request_permissions_tool.spec,
|
||||
create_request_permissions_tool()
|
||||
create_request_permissions_tool(request_permissions_tool_description())
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1205,10 +1235,10 @@ fn web_search_config_is_forwarded_to_tool_spec() {
|
||||
external_web_access: Some(true),
|
||||
filters: web_search_config
|
||||
.filters
|
||||
.map(crate::client_common::tools::ResponsesApiWebSearchFilters::from),
|
||||
.map(ResponsesApiWebSearchFilters::from),
|
||||
user_location: web_search_config
|
||||
.user_location
|
||||
.map(crate::client_common::tools::ResponsesApiWebSearchUserLocation::from),
|
||||
.map(ResponsesApiWebSearchUserLocation::from),
|
||||
search_context_size: web_search_config.search_context_size,
|
||||
search_content_types: None,
|
||||
}
|
||||
@@ -1684,11 +1714,7 @@ fn test_test_model_info_includes_sync_tool() {
|
||||
)
|
||||
.build();
|
||||
|
||||
assert!(
|
||||
tools
|
||||
.iter()
|
||||
.any(|tool| tool_name(&tool.spec) == "test_sync_tool")
|
||||
);
|
||||
assert!(tools.iter().any(|tool| tool.name() == "test_sync_tool"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1822,7 +1848,7 @@ fn test_build_specs_mcp_tools_sorted_by_name() {
|
||||
// Only assert that the MCP tools themselves are sorted by fully-qualified name.
|
||||
let mcp_names: Vec<_> = tools
|
||||
.iter()
|
||||
.map(|t| tool_name(&t.spec).to_string())
|
||||
.map(|t| t.name().to_string())
|
||||
.filter(|n| n.starts_with("test_server/"))
|
||||
.collect();
|
||||
let expected = vec![
|
||||
@@ -2069,7 +2095,7 @@ fn tool_suggest_is_not_registered_without_feature_flag() {
|
||||
assert!(
|
||||
!tools
|
||||
.iter()
|
||||
.any(|tool| tool_name(&tool.spec) == TOOL_SUGGEST_TOOL_NAME)
|
||||
.any(|tool| tool.name() == TOOL_SUGGEST_TOOL_NAME)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2160,7 +2186,7 @@ fn tool_suggest_requires_apps_and_plugins_features() {
|
||||
assert!(
|
||||
!tools
|
||||
.iter()
|
||||
.any(|tool| tool_name(&tool.spec) == TOOL_SUGGEST_TOOL_NAME),
|
||||
.any(|tool| tool.name() == TOOL_SUGGEST_TOOL_NAME),
|
||||
"tool_suggest should be absent when {disabled_feature:?} is disabled"
|
||||
);
|
||||
}
|
||||
@@ -2195,7 +2221,8 @@ fn search_tool_description_handles_no_enabled_apps() {
|
||||
panic!("expected tool_search tool");
|
||||
};
|
||||
|
||||
assert!(description.contains("None currently enabled."));
|
||||
assert!(description.contains("agents"));
|
||||
assert!(description.contains("Agent collaboration tools for spawning, messaging, waiting on, listing, and closing subagents."));
|
||||
assert!(!description.contains("{{app_descriptions}}"));
|
||||
}
|
||||
|
||||
@@ -2653,177 +2680,6 @@ fn test_mcp_tool_anyof_defaults_to_string() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_shell_tool() {
|
||||
let tool = super::create_shell_tool(/*exec_permission_approvals_enabled*/ false);
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
description, name, ..
|
||||
}) = &tool
|
||||
else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert_eq!(name, "shell");
|
||||
|
||||
let expected = if cfg!(windows) {
|
||||
r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"].
|
||||
|
||||
Examples of valid command strings:
|
||||
|
||||
- ls -a (show hidden): ["powershell.exe", "-Command", "Get-ChildItem -Force"]
|
||||
- recursive find by name: ["powershell.exe", "-Command", "Get-ChildItem -Recurse -Filter *.py"]
|
||||
- recursive grep: ["powershell.exe", "-Command", "Get-ChildItem -Path C:\\myrepo -Recurse | Select-String -Pattern 'TODO' -CaseSensitive"]
|
||||
- ps aux | grep python: ["powershell.exe", "-Command", "Get-Process | Where-Object { $_.ProcessName -like '*python*' }"]
|
||||
- setting an env var: ["powershell.exe", "-Command", "$env:FOO='bar'; echo $env:FOO"]
|
||||
- running an inline Python script: ["powershell.exe", "-Command", "@'\\nprint('Hello, world!')\\n'@ | python -"]"#
|
||||
.to_string()
|
||||
+ &windows_shell_safety_description()
|
||||
} else {
|
||||
r#"Runs a shell command and returns its output.
|
||||
- The arguments to `shell` will be passed to execvp(). Most terminal commands should be prefixed with ["bash", "-lc"].
|
||||
- Always set the `workdir` param when using the shell function. Do not use `cd` unless absolutely necessary."#
|
||||
.to_string()
|
||||
};
|
||||
assert_eq!(description, &expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_exec_command_tool_windows_description_includes_shell_safety_guidance() {
|
||||
let tool = super::create_exec_command_tool(
|
||||
/*allow_login_shell*/ true, /*exec_permission_approvals_enabled*/ false,
|
||||
);
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
description, name, ..
|
||||
}) = &tool
|
||||
else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert_eq!(name, "exec_command");
|
||||
|
||||
let expected = if cfg!(windows) {
|
||||
format!(
|
||||
"Runs a command in a PTY, returning output or a session ID for ongoing interaction.{}",
|
||||
windows_shell_safety_description()
|
||||
)
|
||||
} else {
|
||||
"Runs a command in a PTY, returning output or a session ID for ongoing interaction."
|
||||
.to_string()
|
||||
};
|
||||
assert_eq!(description, &expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_tool_with_request_permission_includes_additional_permissions() {
|
||||
let tool = super::create_shell_tool(/*exec_permission_approvals_enabled*/ true);
|
||||
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
let JsonSchema::Object { properties, .. } = parameters else {
|
||||
panic!("expected object parameters");
|
||||
};
|
||||
|
||||
assert!(properties.contains_key("additional_permissions"));
|
||||
|
||||
let Some(JsonSchema::String {
|
||||
description: Some(description),
|
||||
}) = properties.get("sandbox_permissions")
|
||||
else {
|
||||
panic!("expected sandbox_permissions description");
|
||||
};
|
||||
assert!(description.contains("with_additional_permissions"));
|
||||
assert!(description.contains("filesystem or network permissions"));
|
||||
|
||||
let Some(JsonSchema::Object {
|
||||
properties: additional_properties,
|
||||
..
|
||||
}) = properties.get("additional_permissions")
|
||||
else {
|
||||
panic!("expected additional_permissions schema");
|
||||
};
|
||||
assert!(additional_properties.contains_key("network"));
|
||||
assert!(additional_properties.contains_key("file_system"));
|
||||
assert!(!additional_properties.contains_key("macos"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_permissions_tool_includes_full_permission_schema() {
|
||||
let tool = super::create_request_permissions_tool();
|
||||
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
let JsonSchema::Object { properties, .. } = parameters else {
|
||||
panic!("expected object parameters");
|
||||
};
|
||||
let Some(JsonSchema::Object {
|
||||
properties: permission_properties,
|
||||
additional_properties,
|
||||
..
|
||||
}) = properties.get("permissions")
|
||||
else {
|
||||
panic!("expected permissions object");
|
||||
};
|
||||
|
||||
assert_eq!(additional_properties, &Some(false.into()));
|
||||
assert!(permission_properties.contains_key("network"));
|
||||
assert!(permission_properties.contains_key("file_system"));
|
||||
assert!(!permission_properties.contains_key("macos"));
|
||||
|
||||
let Some(JsonSchema::Object {
|
||||
properties: network_properties,
|
||||
additional_properties,
|
||||
..
|
||||
}) = permission_properties.get("network")
|
||||
else {
|
||||
panic!("expected network object");
|
||||
};
|
||||
assert_eq!(additional_properties, &Some(false.into()));
|
||||
assert!(network_properties.contains_key("enabled"));
|
||||
|
||||
let Some(JsonSchema::Object {
|
||||
properties: file_system_properties,
|
||||
additional_properties,
|
||||
..
|
||||
}) = permission_properties.get("file_system")
|
||||
else {
|
||||
panic!("expected file_system object");
|
||||
};
|
||||
assert_eq!(additional_properties, &Some(false.into()));
|
||||
assert!(file_system_properties.contains_key("read"));
|
||||
assert!(file_system_properties.contains_key("write"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_shell_command_tool() {
|
||||
let tool = super::create_shell_command_tool(
|
||||
/*allow_login_shell*/ true, /*exec_permission_approvals_enabled*/ false,
|
||||
);
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
description, name, ..
|
||||
}) = &tool
|
||||
else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert_eq!(name, "shell_command");
|
||||
|
||||
let expected = if cfg!(windows) {
|
||||
r#"Runs a Powershell command (Windows) and returns its output.
|
||||
|
||||
Examples of valid command strings:
|
||||
|
||||
- ls -a (show hidden): "Get-ChildItem -Force"
|
||||
- recursive find by name: "Get-ChildItem -Recurse -Filter *.py"
|
||||
- recursive grep: "Get-ChildItem -Path C:\\myrepo -Recurse | Select-String -Pattern 'TODO' -CaseSensitive"
|
||||
- ps aux | grep python: "Get-Process | Where-Object { $_.ProcessName -like '*python*' }"
|
||||
- setting an env var: "$env:FOO='bar'; echo $env:FOO"
|
||||
- running an inline Python script: "@'\\nprint('Hello, world!')\\n'@ | python -""#
|
||||
.to_string()
|
||||
+ &windows_shell_safety_description()
|
||||
} else {
|
||||
r#"Runs a shell command and returns its output.
|
||||
- Always set the `workdir` param when using the shell_command function. Do not use `cd` unless absolutely necessary."#.to_string()
|
||||
};
|
||||
assert_eq!(description, &expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() {
|
||||
let config = test_config();
|
||||
@@ -3111,38 +2967,3 @@ fn code_mode_exec_description_omits_nested_tool_details_when_not_code_mode_only(
|
||||
assert!(!description.contains("### `update_plan` (`update_plan`)"));
|
||||
assert!(!description.contains("### `view_image` (`view_image`)"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn chat_tools_include_top_level_name() {
|
||||
let properties =
|
||||
BTreeMap::from([("foo".to_string(), JsonSchema::String { description: None })]);
|
||||
let tools = vec![ToolSpec::Function(ResponsesApiTool {
|
||||
name: "demo".to_string(),
|
||||
description: "A demo tool".to_string(),
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::Object {
|
||||
properties,
|
||||
required: None,
|
||||
additional_properties: None,
|
||||
},
|
||||
output_schema: None,
|
||||
})];
|
||||
|
||||
let responses_json = create_tools_json_for_responses_api(&tools).unwrap();
|
||||
assert_eq!(
|
||||
responses_json,
|
||||
vec![json!({
|
||||
"type": "function",
|
||||
"name": "demo",
|
||||
"description": "A demo tool",
|
||||
"strict": false,
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"foo": { "type": "string" }
|
||||
},
|
||||
},
|
||||
})]
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user