mirror of
https://github.com/openai/codex.git
synced 2026-04-06 07:31:37 +03:00
Compare commits
4 Commits
pr16640
...
codex/name
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fcf3dd0270 | ||
|
|
c7c4fa6ab3 | ||
|
|
b5edeb98a0 | ||
|
|
152b676597 |
@@ -1,6 +1,8 @@
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
use crate::PUBLIC_TOOL_NAME;
|
||||
|
||||
@@ -55,6 +57,12 @@ pub struct ToolDefinition {
|
||||
pub output_schema: Option<JsonValue>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub struct ToolNamespaceDescription {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize, PartialEq, Eq)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
struct CodeModeExecPragma {
|
||||
@@ -161,6 +169,7 @@ pub fn is_code_mode_nested_tool(tool_name: &str) -> bool {
|
||||
|
||||
pub fn build_exec_tool_description(
|
||||
enabled_tools: &[(String, String)],
|
||||
namespace_descriptions: &BTreeMap<String, ToolNamespaceDescription>,
|
||||
code_mode_only: bool,
|
||||
) -> String {
|
||||
if !code_mode_only {
|
||||
@@ -173,17 +182,32 @@ pub fn build_exec_tool_description(
|
||||
];
|
||||
|
||||
if !enabled_tools.is_empty() {
|
||||
let nested_tool_reference = enabled_tools
|
||||
.iter()
|
||||
.map(|(name, nested_description)| {
|
||||
let global_name = normalize_code_mode_identifier(name);
|
||||
format!(
|
||||
"### `{global_name}` (`{name}`)\n{}",
|
||||
nested_description.trim()
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n\n");
|
||||
let mut seen_namespaces = BTreeSet::new();
|
||||
let mut nested_tool_sections = Vec::with_capacity(enabled_tools.len());
|
||||
|
||||
for (name, nested_description) in enabled_tools {
|
||||
if let Some(namespace_description) = namespace_descriptions.get(name)
|
||||
&& seen_namespaces.insert(namespace_description.name.clone())
|
||||
{
|
||||
nested_tool_sections.push(format!(
|
||||
"## {}\n{}",
|
||||
namespace_description.name,
|
||||
namespace_description.description.trim()
|
||||
));
|
||||
}
|
||||
|
||||
let global_name = normalize_code_mode_identifier(name);
|
||||
let nested_description = nested_description.trim();
|
||||
if nested_description.is_empty() {
|
||||
nested_tool_sections.push(format!("### `{global_name}` (`{name}`)"));
|
||||
} else {
|
||||
nested_tool_sections.push(format!(
|
||||
"### `{global_name}` (`{name}`)\n{nested_description}"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
let nested_tool_reference = nested_tool_sections.join("\n\n");
|
||||
sections.push(nested_tool_reference);
|
||||
}
|
||||
|
||||
@@ -265,7 +289,7 @@ fn append_code_mode_sample_for_definition(definition: &ToolDefinition) -> String
|
||||
CodeModeToolKind::Function => definition
|
||||
.input_schema
|
||||
.as_ref()
|
||||
.map(render_json_schema_to_typescript)
|
||||
.map(render_json_schema_to_typescript_with_property_descriptions)
|
||||
.unwrap_or_else(|| "unknown".to_string()),
|
||||
CodeModeToolKind::Freeform => "string".to_string(),
|
||||
};
|
||||
@@ -297,6 +321,33 @@ pub fn render_json_schema_to_typescript(schema: &JsonValue) -> String {
|
||||
render_json_schema_to_typescript_inner(schema)
|
||||
}
|
||||
|
||||
fn render_json_schema_to_typescript_with_property_descriptions(schema: &JsonValue) -> String {
|
||||
let Some(map) = schema.as_object() else {
|
||||
return render_json_schema_to_typescript(schema);
|
||||
};
|
||||
|
||||
if !(map.contains_key("properties")
|
||||
|| map.contains_key("additionalProperties")
|
||||
|| map.contains_key("required"))
|
||||
{
|
||||
return render_json_schema_to_typescript(schema);
|
||||
}
|
||||
|
||||
let Some(properties) = map.get("properties").and_then(JsonValue::as_object) else {
|
||||
return render_json_schema_to_typescript(schema);
|
||||
};
|
||||
if properties.values().all(|value| {
|
||||
value
|
||||
.get("description")
|
||||
.and_then(JsonValue::as_str)
|
||||
.is_none_or(str::is_empty)
|
||||
}) {
|
||||
return render_json_schema_to_typescript(schema);
|
||||
}
|
||||
|
||||
render_json_schema_object_with_property_descriptions(map)
|
||||
}
|
||||
|
||||
fn render_json_schema_to_typescript_inner(schema: &JsonValue) -> String {
|
||||
match schema {
|
||||
JsonValue::Bool(true) => "unknown".to_string(),
|
||||
@@ -460,6 +511,67 @@ fn render_json_schema_object(map: &serde_json::Map<String, JsonValue>) -> String
|
||||
format!("{{ {} }}", lines.join(" "))
|
||||
}
|
||||
|
||||
fn render_json_schema_object_with_property_descriptions(
|
||||
map: &serde_json::Map<String, JsonValue>,
|
||||
) -> String {
|
||||
let required = map
|
||||
.get("required")
|
||||
.and_then(JsonValue::as_array)
|
||||
.map(|items| {
|
||||
items
|
||||
.iter()
|
||||
.filter_map(JsonValue::as_str)
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
let properties = map
|
||||
.get("properties")
|
||||
.and_then(JsonValue::as_object)
|
||||
.cloned()
|
||||
.unwrap_or_default();
|
||||
|
||||
let mut sorted_properties = properties.iter().collect::<Vec<_>>();
|
||||
sorted_properties.sort_unstable_by(|(name_a, _), (name_b, _)| name_a.cmp(name_b));
|
||||
let mut lines = vec!["{".to_string()];
|
||||
for (name, value) in sorted_properties {
|
||||
if let Some(description) = value.get("description").and_then(JsonValue::as_str) {
|
||||
for description_line in description
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|line| !line.is_empty())
|
||||
{
|
||||
lines.push(format!(" // {description_line}"));
|
||||
}
|
||||
}
|
||||
|
||||
let optional = if required.iter().any(|required_name| required_name == name) {
|
||||
""
|
||||
} else {
|
||||
"?"
|
||||
};
|
||||
let property_name = render_json_schema_property_name(name);
|
||||
let property_type = render_json_schema_to_typescript_inner(value);
|
||||
lines.push(format!(" {property_name}{optional}: {property_type};"));
|
||||
}
|
||||
|
||||
if let Some(additional_properties) = map.get("additionalProperties") {
|
||||
let property_type = match additional_properties {
|
||||
JsonValue::Bool(true) => Some("unknown".to_string()),
|
||||
JsonValue::Bool(false) => None,
|
||||
value => Some(render_json_schema_to_typescript_inner(value)),
|
||||
};
|
||||
|
||||
if let Some(property_type) = property_type {
|
||||
lines.push(format!(" [key: string]: {property_type};"));
|
||||
}
|
||||
} else if properties.is_empty() {
|
||||
lines.push(" [key: string]: unknown;".to_string());
|
||||
}
|
||||
|
||||
lines.push("}".to_string());
|
||||
lines.join("\n")
|
||||
}
|
||||
|
||||
fn render_json_schema_property_name(name: &str) -> String {
|
||||
if normalize_code_mode_identifier(name) == name {
|
||||
name.to_string()
|
||||
@@ -477,12 +589,14 @@ mod tests {
|
||||
use super::CodeModeToolKind;
|
||||
use super::ParsedExecSource;
|
||||
use super::ToolDefinition;
|
||||
use super::ToolNamespaceDescription;
|
||||
use super::augment_tool_definition;
|
||||
use super::build_exec_tool_description;
|
||||
use super::normalize_code_mode_identifier;
|
||||
use super::parse_exec_source;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
#[test]
|
||||
fn parse_exec_source_without_pragma() {
|
||||
@@ -548,12 +662,75 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn augment_tool_definition_includes_input_property_descriptions_as_comments() {
|
||||
let definition = ToolDefinition {
|
||||
name: "weather_tool".to_string(),
|
||||
description: "Weather tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"weather": {
|
||||
"type": "array",
|
||||
"description": "look up weather for a given list of locations",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"location": { "type": "string" }
|
||||
},
|
||||
"required": ["location"]
|
||||
}
|
||||
}
|
||||
}
|
||||
})),
|
||||
output_schema: None,
|
||||
};
|
||||
|
||||
let description = augment_tool_definition(definition).description;
|
||||
assert!(description.contains("// look up weather for a given list of locations"));
|
||||
assert!(description.contains("weather?: Array<{ location: string; }>;"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_only_description_includes_nested_tools() {
|
||||
let description = build_exec_tool_description(
|
||||
&[("foo".to_string(), "bar".to_string())],
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ true,
|
||||
);
|
||||
assert!(description.contains("### `foo` (`foo`)"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_only_description_groups_namespace_instructions_once() {
|
||||
let namespace_descriptions = BTreeMap::from([
|
||||
(
|
||||
"mcp__sample__alpha".to_string(),
|
||||
ToolNamespaceDescription {
|
||||
name: "mcp__sample".to_string(),
|
||||
description: "Shared namespace guidance.".to_string(),
|
||||
},
|
||||
),
|
||||
(
|
||||
"mcp__sample__beta".to_string(),
|
||||
ToolNamespaceDescription {
|
||||
name: "mcp__sample".to_string(),
|
||||
description: "Shared namespace guidance.".to_string(),
|
||||
},
|
||||
),
|
||||
]);
|
||||
let description = build_exec_tool_description(
|
||||
&[
|
||||
("mcp__sample__alpha".to_string(), "First tool".to_string()),
|
||||
("mcp__sample__beta".to_string(), "Second tool".to_string()),
|
||||
],
|
||||
&namespace_descriptions,
|
||||
/*code_mode_only*/ true,
|
||||
);
|
||||
assert_eq!(description.matches("## mcp__sample").count(), 1);
|
||||
assert_eq!(description.matches("Shared namespace guidance.").count(), 1);
|
||||
assert!(description.contains("### `mcp__sample__alpha` (`mcp__sample__alpha`)"));
|
||||
assert!(description.contains("### `mcp__sample__beta` (`mcp__sample__beta`)"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ mod service;
|
||||
pub use description::CODE_MODE_PRAGMA_PREFIX;
|
||||
pub use description::CodeModeToolKind;
|
||||
pub use description::ToolDefinition;
|
||||
pub use description::ToolNamespaceDescription;
|
||||
pub use description::append_code_mode_sample;
|
||||
pub use description::augment_tool_definition;
|
||||
pub use description::build_exec_tool_description;
|
||||
|
||||
@@ -43,7 +43,6 @@ use crate::state::TaskKind;
|
||||
use crate::tasks::SessionTask;
|
||||
use crate::tasks::SessionTaskContext;
|
||||
use crate::tools::ToolRouter;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::handlers::ShellHandler;
|
||||
@@ -120,12 +119,6 @@ use std::time::Duration as StdDuration;
|
||||
#[path = "codex_tests_guardian.rs"]
|
||||
mod guardian_tests;
|
||||
|
||||
use codex_protocol::models::function_call_output_content_items_to_text;
|
||||
|
||||
fn expect_text_tool_output(output: &FunctionToolOutput) -> String {
|
||||
function_call_output_content_items_to_text(&output.body).unwrap_or_default()
|
||||
}
|
||||
|
||||
struct InstructionsTestCase {
|
||||
slug: &'static str,
|
||||
expects_apply_patch_instructions: bool,
|
||||
@@ -5348,7 +5341,9 @@ async fn sample_rollout(
|
||||
#[tokio::test]
|
||||
async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
@@ -5394,23 +5389,6 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
let params2 = ExecParams {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
command: params.command.clone(),
|
||||
cwd: params.cwd.clone(),
|
||||
expiration: timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: params.justification.clone(),
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
|
||||
let tool_name = "shell";
|
||||
@@ -5448,9 +5426,11 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
);
|
||||
|
||||
pretty_assertions::assert_eq!(output, expected);
|
||||
pretty_assertions::assert_eq!(session.granted_turn_permissions().await, None);
|
||||
|
||||
// Now retry the same command WITHOUT escalated permissions; should succeed.
|
||||
// Force DangerFullAccess to avoid platform sandbox dependencies in tests.
|
||||
// The rejection should not poison the non-escalated path for the same
|
||||
// command. Force DangerFullAccess so this check stays focused on approval
|
||||
// policy rather than platform-specific sandbox behavior.
|
||||
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("unique turn context Arc");
|
||||
turn_context_mut
|
||||
.sandbox_policy
|
||||
@@ -5461,45 +5441,22 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
turn_context_mut.network_sandbox_policy =
|
||||
NetworkSandboxPolicy::from(turn_context_mut.sandbox_policy.get());
|
||||
|
||||
let resp2 = handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
tracker: Arc::clone(&turn_diff_tracker),
|
||||
call_id: "test-call-2".to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
tool_namespace: None,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"command": params2.command.clone(),
|
||||
"workdir": Some(turn_context.cwd.to_string_lossy().to_string()),
|
||||
"timeout_ms": params2.expiration.timeout_ms(),
|
||||
"sandbox_permissions": params2.sandbox_permissions,
|
||||
"justification": params2.justification.clone(),
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
let exec_approval_requirement = session
|
||||
.services
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: ¶ms.command,
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
sandbox_policy: turn_context.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &turn_context.file_system_sandbox_policy,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let output = expect_text_tool_output(&resp2.expect("expected Ok result"));
|
||||
|
||||
#[derive(Deserialize, PartialEq, Eq, Debug)]
|
||||
struct ResponseExecMetadata {
|
||||
exit_code: i32,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct ResponseExecOutput {
|
||||
output: String,
|
||||
metadata: ResponseExecMetadata,
|
||||
}
|
||||
|
||||
let exec_output: ResponseExecOutput =
|
||||
serde_json::from_str(&output).expect("valid exec output json");
|
||||
|
||||
pretty_assertions::assert_eq!(exec_output.metadata, ResponseExecMetadata { exit_code: 0 });
|
||||
assert!(exec_output.output.contains("hi"));
|
||||
assert!(matches!(
|
||||
exec_approval_requirement,
|
||||
ExecApprovalRequirement::Skip { .. }
|
||||
));
|
||||
}
|
||||
#[tokio::test]
|
||||
async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
|
||||
@@ -23,6 +23,14 @@ use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::matchers::header;
|
||||
|
||||
fn normalize_git_remote_url(url: &str) -> String {
|
||||
let normalized = url.trim().trim_end_matches('/');
|
||||
normalized
|
||||
.strip_suffix(".git")
|
||||
.unwrap_or(normalized)
|
||||
.to_string()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn responses_stream_includes_subagent_header_on_review() {
|
||||
core_test_support::skip_if_no_network!();
|
||||
@@ -540,13 +548,15 @@ async fn responses_stream_includes_turn_metadata_header_for_git_workspace_e2e()
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some(expected_head.as_str())
|
||||
);
|
||||
let actual_origin = workspace
|
||||
.get("associated_remote_urls")
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.and_then(|remotes| remotes.get("origin"))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.expect("origin remote should be present");
|
||||
assert_eq!(
|
||||
workspace
|
||||
.get("associated_remote_urls")
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.and_then(|remotes| remotes.get("origin"))
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some(expected_origin.as_str())
|
||||
normalize_git_remote_url(actual_origin),
|
||||
normalize_git_remote_url(&expected_origin)
|
||||
);
|
||||
assert_eq!(
|
||||
workspace
|
||||
|
||||
@@ -102,6 +102,7 @@ pub fn create_wait_tool() -> ToolSpec {
|
||||
|
||||
pub fn create_code_mode_tool(
|
||||
enabled_tools: &[(String, String)],
|
||||
namespace_descriptions: &BTreeMap<String, codex_code_mode::ToolNamespaceDescription>,
|
||||
code_mode_only_enabled: bool,
|
||||
) -> ToolSpec {
|
||||
const CODE_MODE_FREEFORM_GRAMMAR: &str = r#"
|
||||
@@ -118,6 +119,7 @@ SOURCE: /[\s\S]+/
|
||||
name: codex_code_mode::PUBLIC_TOOL_NAME.to_string(),
|
||||
description: codex_code_mode::build_exec_tool_description(
|
||||
enabled_tools,
|
||||
namespace_descriptions,
|
||||
code_mode_only_enabled,
|
||||
),
|
||||
format: FreeformToolFormat {
|
||||
|
||||
@@ -185,11 +185,16 @@ fn create_code_mode_tool_matches_expected_spec() {
|
||||
let enabled_tools = vec![("update_plan".to_string(), "Update the plan".to_string())];
|
||||
|
||||
assert_eq!(
|
||||
create_code_mode_tool(&enabled_tools, /*code_mode_only_enabled*/ true),
|
||||
create_code_mode_tool(
|
||||
&enabled_tools,
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only_enabled*/ true,
|
||||
),
|
||||
ToolSpec::Freeform(FreeformTool {
|
||||
name: codex_code_mode::PUBLIC_TOOL_NAME.to_string(),
|
||||
description: codex_code_mode::build_exec_tool_description(
|
||||
&enabled_tools,
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ true
|
||||
),
|
||||
format: FreeformToolFormat {
|
||||
|
||||
@@ -1,48 +1,60 @@
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::Serializer;
|
||||
use serde_json::Value as JsonValue;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
/// Generic JSON-Schema subset needed for our tool definitions.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
#[serde(tag = "type", rename_all = "lowercase")]
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub enum JsonSchema {
|
||||
Boolean {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
description: Option<String>,
|
||||
},
|
||||
String {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
description: Option<String>,
|
||||
},
|
||||
/// MCP schema allows "number" | "integer" for Number.
|
||||
#[serde(alias = "integer")]
|
||||
Number {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
description: Option<String>,
|
||||
},
|
||||
Null {
|
||||
description: Option<String>,
|
||||
},
|
||||
Array {
|
||||
items: Box<JsonSchema>,
|
||||
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
description: Option<String>,
|
||||
},
|
||||
Object {
|
||||
properties: BTreeMap<String, JsonSchema>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
required: Option<Vec<String>>,
|
||||
#[serde(
|
||||
rename = "additionalProperties",
|
||||
skip_serializing_if = "Option::is_none"
|
||||
)]
|
||||
additional_properties: Option<AdditionalProperties>,
|
||||
},
|
||||
Const {
|
||||
value: JsonValue,
|
||||
schema_type: Option<String>,
|
||||
description: Option<String>,
|
||||
},
|
||||
Enum {
|
||||
values: Vec<JsonValue>,
|
||||
schema_type: Option<String>,
|
||||
description: Option<String>,
|
||||
},
|
||||
AnyOf {
|
||||
variants: Vec<JsonSchema>,
|
||||
description: Option<String>,
|
||||
},
|
||||
OneOf {
|
||||
variants: Vec<JsonSchema>,
|
||||
description: Option<String>,
|
||||
},
|
||||
AllOf {
|
||||
variants: Vec<JsonSchema>,
|
||||
description: Option<String>,
|
||||
},
|
||||
}
|
||||
|
||||
/// Whether additional properties are allowed, and if so, any required schema.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
#[serde(untagged)]
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub enum AdditionalProperties {
|
||||
Boolean(bool),
|
||||
Schema(Box<JsonSchema>),
|
||||
@@ -60,18 +72,41 @@ impl From<JsonSchema> for AdditionalProperties {
|
||||
}
|
||||
}
|
||||
|
||||
impl Serialize for JsonSchema {
|
||||
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: Serializer,
|
||||
{
|
||||
json_schema_to_json(self).serialize(serializer)
|
||||
}
|
||||
}
|
||||
|
||||
impl Serialize for AdditionalProperties {
|
||||
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: Serializer,
|
||||
{
|
||||
match self {
|
||||
Self::Boolean(value) => value.serialize(serializer),
|
||||
Self::Schema(schema) => json_schema_to_json(schema).serialize(serializer),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse the tool `input_schema` or return an error for invalid schema.
|
||||
pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result<JsonSchema, serde_json::Error> {
|
||||
let mut input_schema = input_schema.clone();
|
||||
sanitize_json_schema(&mut input_schema);
|
||||
serde_json::from_value::<JsonSchema>(input_schema)
|
||||
parse_json_schema(&input_schema)
|
||||
}
|
||||
|
||||
/// Sanitize a JSON Schema (as serde_json::Value) so it can fit our limited
|
||||
/// JsonSchema enum. This function:
|
||||
/// - Ensures every schema object has a "type". If missing, infers it from
|
||||
/// common keywords (properties => object, items => array, enum/const/format => string)
|
||||
/// and otherwise defaults to "string".
|
||||
/// - Infers a concrete `"type"` when it is missing and the shape can be reduced
|
||||
/// to our supported subset (properties => object, items => array,
|
||||
/// enum/const/format => string).
|
||||
/// - Preserves explicit combiners like `anyOf`/`oneOf`/`allOf` and nullable
|
||||
/// unions instead of collapsing them to a single fallback type.
|
||||
/// - Fills required child fields (e.g. array items, object properties) with
|
||||
/// permissive defaults when absent.
|
||||
fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
@@ -107,22 +142,6 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
.and_then(|value| value.as_str())
|
||||
.map(str::to_string);
|
||||
|
||||
if schema_type.is_none()
|
||||
&& let Some(JsonValue::Array(types)) = map.get("type")
|
||||
{
|
||||
for candidate in types {
|
||||
if let Some(candidate_type) = candidate.as_str()
|
||||
&& matches!(
|
||||
candidate_type,
|
||||
"object" | "array" | "string" | "number" | "integer" | "boolean"
|
||||
)
|
||||
{
|
||||
schema_type = Some(candidate_type.to_string());
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if schema_type.is_none() {
|
||||
if map.contains_key("properties")
|
||||
|| map.contains_key("required")
|
||||
@@ -146,10 +165,11 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
}
|
||||
}
|
||||
|
||||
let schema_type = schema_type.unwrap_or_else(|| "string".to_string());
|
||||
map.insert("type".to_string(), JsonValue::String(schema_type.clone()));
|
||||
if let Some(schema_type) = &schema_type {
|
||||
map.insert("type".to_string(), JsonValue::String(schema_type.clone()));
|
||||
}
|
||||
|
||||
if schema_type == "object" {
|
||||
if schema_type.as_deref() == Some("object") {
|
||||
if !map.contains_key("properties") {
|
||||
map.insert(
|
||||
"properties".to_string(),
|
||||
@@ -163,7 +183,7 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
}
|
||||
}
|
||||
|
||||
if schema_type == "array" && !map.contains_key("items") {
|
||||
if schema_type.as_deref() == Some("array") && !map.contains_key("items") {
|
||||
map.insert("items".to_string(), json!({ "type": "string" }));
|
||||
}
|
||||
}
|
||||
@@ -171,6 +191,284 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_json_schema(value: &JsonValue) -> Result<JsonSchema, serde_json::Error> {
|
||||
match value {
|
||||
JsonValue::Bool(_) => Ok(JsonSchema::String { description: None }),
|
||||
JsonValue::Object(map) => {
|
||||
let description = map
|
||||
.get("description")
|
||||
.and_then(JsonValue::as_str)
|
||||
.map(str::to_string);
|
||||
|
||||
if let Some(value) = map.get("const") {
|
||||
return Ok(JsonSchema::Const {
|
||||
value: value.clone(),
|
||||
schema_type: map
|
||||
.get("type")
|
||||
.and_then(JsonValue::as_str)
|
||||
.map(str::to_string),
|
||||
description,
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(values) = map.get("enum").and_then(JsonValue::as_array) {
|
||||
return Ok(JsonSchema::Enum {
|
||||
values: values.clone(),
|
||||
schema_type: map
|
||||
.get("type")
|
||||
.and_then(JsonValue::as_str)
|
||||
.map(str::to_string),
|
||||
description,
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(variants) = map.get("anyOf").and_then(JsonValue::as_array) {
|
||||
return Ok(JsonSchema::AnyOf {
|
||||
variants: variants
|
||||
.iter()
|
||||
.map(parse_json_schema)
|
||||
.collect::<Result<Vec<_>, _>>()?,
|
||||
description,
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(variants) = map.get("oneOf").and_then(JsonValue::as_array) {
|
||||
return Ok(JsonSchema::OneOf {
|
||||
variants: variants
|
||||
.iter()
|
||||
.map(parse_json_schema)
|
||||
.collect::<Result<Vec<_>, _>>()?,
|
||||
description,
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(variants) = map.get("allOf").and_then(JsonValue::as_array) {
|
||||
return Ok(JsonSchema::AllOf {
|
||||
variants: variants
|
||||
.iter()
|
||||
.map(parse_json_schema)
|
||||
.collect::<Result<Vec<_>, _>>()?,
|
||||
description,
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(types) = map.get("type").and_then(JsonValue::as_array) {
|
||||
return Ok(JsonSchema::AnyOf {
|
||||
variants: types
|
||||
.iter()
|
||||
.filter_map(JsonValue::as_str)
|
||||
.map(|schema_type| {
|
||||
parse_json_schema(&json!({
|
||||
"type": schema_type,
|
||||
}))
|
||||
})
|
||||
.collect::<Result<Vec<_>, _>>()?,
|
||||
description,
|
||||
});
|
||||
}
|
||||
|
||||
match map
|
||||
.get("type")
|
||||
.and_then(JsonValue::as_str)
|
||||
.unwrap_or("string")
|
||||
{
|
||||
"boolean" => Ok(JsonSchema::Boolean { description }),
|
||||
"string" => Ok(JsonSchema::String { description }),
|
||||
"number" | "integer" => Ok(JsonSchema::Number { description }),
|
||||
"null" => Ok(JsonSchema::Null { description }),
|
||||
"array" => Ok(JsonSchema::Array {
|
||||
items: Box::new(parse_json_schema(
|
||||
map.get("items").unwrap_or(&json!({ "type": "string" })),
|
||||
)?),
|
||||
description,
|
||||
}),
|
||||
"object" => {
|
||||
let properties = map
|
||||
.get("properties")
|
||||
.and_then(JsonValue::as_object)
|
||||
.cloned()
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.map(|(name, value)| Ok((name, parse_json_schema(&value)?)))
|
||||
.collect::<Result<BTreeMap<_, _>, serde_json::Error>>()?;
|
||||
let required = map
|
||||
.get("required")
|
||||
.and_then(JsonValue::as_array)
|
||||
.map(|items| {
|
||||
items
|
||||
.iter()
|
||||
.filter_map(JsonValue::as_str)
|
||||
.map(str::to_string)
|
||||
.collect::<Vec<_>>()
|
||||
});
|
||||
let additional_properties = map
|
||||
.get("additionalProperties")
|
||||
.map(parse_additional_properties)
|
||||
.transpose()?;
|
||||
Ok(JsonSchema::Object {
|
||||
properties,
|
||||
required,
|
||||
additional_properties,
|
||||
})
|
||||
}
|
||||
_ => Ok(JsonSchema::String { description }),
|
||||
}
|
||||
}
|
||||
_ => Ok(JsonSchema::String { description: None }),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_additional_properties(
|
||||
value: &JsonValue,
|
||||
) -> Result<AdditionalProperties, serde_json::Error> {
|
||||
match value {
|
||||
JsonValue::Bool(flag) => Ok(AdditionalProperties::Boolean(*flag)),
|
||||
_ => Ok(AdditionalProperties::Schema(Box::new(parse_json_schema(
|
||||
value,
|
||||
)?))),
|
||||
}
|
||||
}
|
||||
|
||||
fn json_schema_to_json(schema: &JsonSchema) -> JsonValue {
|
||||
match schema {
|
||||
JsonSchema::Boolean { description } => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("type".to_string(), JsonValue::String("boolean".to_string()));
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::String { description } => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("type".to_string(), JsonValue::String("string".to_string()));
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::Number { description } => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("type".to_string(), JsonValue::String("number".to_string()));
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::Null { description } => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("type".to_string(), JsonValue::String("null".to_string()));
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::Array { items, description } => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("type".to_string(), JsonValue::String("array".to_string()));
|
||||
map.insert("items".to_string(), json_schema_to_json(items));
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::Object {
|
||||
properties,
|
||||
required,
|
||||
additional_properties,
|
||||
} => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("type".to_string(), JsonValue::String("object".to_string()));
|
||||
map.insert(
|
||||
"properties".to_string(),
|
||||
JsonValue::Object(
|
||||
properties
|
||||
.iter()
|
||||
.map(|(name, value)| (name.clone(), json_schema_to_json(value)))
|
||||
.collect(),
|
||||
),
|
||||
);
|
||||
if let Some(required) = required {
|
||||
map.insert(
|
||||
"required".to_string(),
|
||||
JsonValue::Array(required.iter().cloned().map(JsonValue::String).collect()),
|
||||
);
|
||||
}
|
||||
if let Some(additional_properties) = additional_properties {
|
||||
map.insert(
|
||||
"additionalProperties".to_string(),
|
||||
match additional_properties {
|
||||
AdditionalProperties::Boolean(flag) => JsonValue::Bool(*flag),
|
||||
AdditionalProperties::Schema(schema) => json_schema_to_json(schema),
|
||||
},
|
||||
);
|
||||
}
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::Const {
|
||||
value,
|
||||
schema_type,
|
||||
description,
|
||||
} => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("const".to_string(), value.clone());
|
||||
if let Some(schema_type) = schema_type {
|
||||
map.insert("type".to_string(), JsonValue::String(schema_type.clone()));
|
||||
}
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::Enum {
|
||||
values,
|
||||
schema_type,
|
||||
description,
|
||||
} => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("enum".to_string(), JsonValue::Array(values.clone()));
|
||||
if let Some(schema_type) = schema_type {
|
||||
map.insert("type".to_string(), JsonValue::String(schema_type.clone()));
|
||||
}
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::AnyOf {
|
||||
variants,
|
||||
description,
|
||||
} => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert(
|
||||
"anyOf".to_string(),
|
||||
JsonValue::Array(variants.iter().map(json_schema_to_json).collect()),
|
||||
);
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::OneOf {
|
||||
variants,
|
||||
description,
|
||||
} => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert(
|
||||
"oneOf".to_string(),
|
||||
JsonValue::Array(variants.iter().map(json_schema_to_json).collect()),
|
||||
);
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
JsonSchema::AllOf {
|
||||
variants,
|
||||
description,
|
||||
} => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert(
|
||||
"allOf".to_string(),
|
||||
JsonValue::Array(variants.iter().map(json_schema_to_json).collect()),
|
||||
);
|
||||
insert_description(&mut map, description.as_deref());
|
||||
JsonValue::Object(map)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn insert_description(map: &mut serde_json::Map<String, JsonValue>, description: Option<&str>) {
|
||||
if let Some(description) = description {
|
||||
map.insert(
|
||||
"description".to_string(),
|
||||
JsonValue::String(description.to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "json_schema_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -87,7 +87,13 @@ fn parse_tool_input_schema_sanitizes_additional_properties_schema() {
|
||||
JsonSchema::Object {
|
||||
properties: BTreeMap::from([(
|
||||
"value".to_string(),
|
||||
JsonSchema::String { description: None },
|
||||
JsonSchema::AnyOf {
|
||||
variants: vec![
|
||||
JsonSchema::String { description: None },
|
||||
JsonSchema::Number { description: None },
|
||||
],
|
||||
description: None,
|
||||
},
|
||||
)]),
|
||||
required: Some(vec!["value".to_string()]),
|
||||
additional_properties: None,
|
||||
@@ -96,3 +102,157 @@ fn parse_tool_input_schema_sanitizes_additional_properties_schema() {
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_input_schema_preserves_web_run_shape() {
|
||||
let schema = parse_tool_input_schema(&serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"open": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ref_id": {"type": "string"},
|
||||
"lineno": {"anyOf": [{"type": "integer"}, {"type": "null"}]}
|
||||
},
|
||||
"required": ["ref_id"],
|
||||
"additionalProperties": false
|
||||
}
|
||||
},
|
||||
{"type": "null"}
|
||||
]
|
||||
},
|
||||
"tagged_list": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"kind": {"type": "const", "const": "tagged"},
|
||||
"variant": {"type": "enum", "enum": ["alpha", "beta"]},
|
||||
"scope": {"type": "enum", "enum": ["one", "two"]}
|
||||
},
|
||||
"required": ["kind", "variant", "scope"]
|
||||
}
|
||||
},
|
||||
{"type": "null"}
|
||||
]
|
||||
},
|
||||
"response_length": {
|
||||
"type": "enum",
|
||||
"enum": ["short", "medium", "long"]
|
||||
}
|
||||
}
|
||||
}))
|
||||
.expect("parse schema");
|
||||
|
||||
assert_eq!(
|
||||
schema,
|
||||
JsonSchema::Object {
|
||||
properties: BTreeMap::from([
|
||||
(
|
||||
"open".to_string(),
|
||||
JsonSchema::AnyOf {
|
||||
variants: vec![
|
||||
JsonSchema::Array {
|
||||
items: Box::new(JsonSchema::Object {
|
||||
properties: BTreeMap::from([
|
||||
(
|
||||
"lineno".to_string(),
|
||||
JsonSchema::AnyOf {
|
||||
variants: vec![
|
||||
JsonSchema::Number { description: None },
|
||||
JsonSchema::Null { description: None },
|
||||
],
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
"ref_id".to_string(),
|
||||
JsonSchema::String { description: None },
|
||||
),
|
||||
]),
|
||||
required: Some(vec!["ref_id".to_string()]),
|
||||
additional_properties: Some(false.into()),
|
||||
}),
|
||||
description: None,
|
||||
},
|
||||
JsonSchema::Null { description: None },
|
||||
],
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
"response_length".to_string(),
|
||||
JsonSchema::Enum {
|
||||
values: vec![
|
||||
serde_json::json!("short"),
|
||||
serde_json::json!("medium"),
|
||||
serde_json::json!("long"),
|
||||
],
|
||||
schema_type: Some("enum".to_string()),
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
"tagged_list".to_string(),
|
||||
JsonSchema::AnyOf {
|
||||
variants: vec![
|
||||
JsonSchema::Array {
|
||||
items: Box::new(JsonSchema::Object {
|
||||
properties: BTreeMap::from([
|
||||
(
|
||||
"kind".to_string(),
|
||||
JsonSchema::Const {
|
||||
value: serde_json::json!("tagged"),
|
||||
schema_type: Some("const".to_string()),
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
"scope".to_string(),
|
||||
JsonSchema::Enum {
|
||||
values: vec![
|
||||
serde_json::json!("one"),
|
||||
serde_json::json!("two"),
|
||||
],
|
||||
schema_type: Some("enum".to_string()),
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
"variant".to_string(),
|
||||
JsonSchema::Enum {
|
||||
values: vec![
|
||||
serde_json::json!("alpha"),
|
||||
serde_json::json!("beta"),
|
||||
],
|
||||
schema_type: Some("enum".to_string()),
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
]),
|
||||
required: Some(vec![
|
||||
"kind".to_string(),
|
||||
"variant".to_string(),
|
||||
"scope".to_string(),
|
||||
]),
|
||||
additional_properties: None,
|
||||
}),
|
||||
description: None,
|
||||
},
|
||||
JsonSchema::Null { description: None },
|
||||
],
|
||||
description: None,
|
||||
},
|
||||
),
|
||||
]),
|
||||
required: None,
|
||||
additional_properties: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -61,15 +61,37 @@ use crate::tool_registry_plan_types::agent_type_description;
|
||||
use codex_protocol::openai_models::ApplyPatchToolType;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use rmcp::model::Tool as McpTool;
|
||||
use serde_json::Value;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
pub fn build_tool_registry_plan(
|
||||
config: &ToolsConfig,
|
||||
params: ToolRegistryPlanParams<'_>,
|
||||
) -> ToolRegistryPlan {
|
||||
const CODEX_NAMESPACE_DESCRIPTION_META_KEY: &str = "codex_namespace_description";
|
||||
const CODEX_NAMESPACE_META_KEY: &str = "codex_namespace";
|
||||
|
||||
let mut plan = ToolRegistryPlan::new();
|
||||
let exec_permission_approvals_enabled = config.exec_permission_approvals_enabled;
|
||||
|
||||
if config.code_mode_enabled {
|
||||
let namespace_descriptions = params
|
||||
.mcp_tools
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.filter_map(|(name, tool)| {
|
||||
let meta = tool.meta.as_ref()?;
|
||||
let namespace = meta_string(meta, CODEX_NAMESPACE_META_KEY)?;
|
||||
let description = meta_string(meta, CODEX_NAMESPACE_DESCRIPTION_META_KEY)?;
|
||||
Some((
|
||||
name.clone(),
|
||||
codex_code_mode::ToolNamespaceDescription {
|
||||
name: namespace,
|
||||
description,
|
||||
},
|
||||
))
|
||||
})
|
||||
.collect::<BTreeMap<_, _>>();
|
||||
let nested_config = config.for_code_mode_nested_tools();
|
||||
let nested_plan = build_tool_registry_plan(
|
||||
&nested_config,
|
||||
@@ -88,7 +110,11 @@ pub fn build_tool_registry_plan(
|
||||
.map(|tool| (tool.name, tool.description))
|
||||
.collect::<Vec<_>>();
|
||||
plan.push_spec(
|
||||
create_code_mode_tool(&enabled_tools, config.code_mode_only_enabled),
|
||||
create_code_mode_tool(
|
||||
&enabled_tools,
|
||||
&namespace_descriptions,
|
||||
config.code_mode_only_enabled,
|
||||
),
|
||||
/*supports_parallel_tool_calls*/ false,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
@@ -487,6 +513,14 @@ pub fn build_tool_registry_plan(
|
||||
plan
|
||||
}
|
||||
|
||||
fn meta_string(meta: &rmcp::model::Meta, key: &str) -> Option<String> {
|
||||
meta.get(key)
|
||||
.and_then(Value::as_str)
|
||||
.map(str::trim)
|
||||
.filter(|value| !value.is_empty())
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "tool_registry_plan_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -28,6 +28,7 @@ use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::Meta;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
@@ -1498,6 +1499,7 @@ fn code_mode_augments_mcp_tool_descriptions_with_namespaced_sample() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::CodeModeOnly);
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
@@ -1543,6 +1545,172 @@ fn code_mode_augments_mcp_tool_descriptions_with_namespaced_sample() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_preserves_nullable_and_literal_mcp_input_shapes() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"mcp__sample__fn".to_string(),
|
||||
mcp_tool(
|
||||
"fn",
|
||||
"Sample fn",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"open": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ref_id": {"type": "string"},
|
||||
"lineno": {"anyOf": [{"type": "integer"}, {"type": "null"}]}
|
||||
},
|
||||
"required": ["ref_id"],
|
||||
"additionalProperties": false
|
||||
}
|
||||
},
|
||||
{"type": "null"}
|
||||
]
|
||||
},
|
||||
"tagged_list": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"kind": {"type": "const", "const": "tagged"},
|
||||
"variant": {"type": "enum", "enum": ["alpha", "beta"]},
|
||||
"scope": {"type": "enum", "enum": ["one", "two"]}
|
||||
},
|
||||
"required": ["kind", "variant", "scope"]
|
||||
}
|
||||
},
|
||||
{"type": "null"}
|
||||
]
|
||||
},
|
||||
"response_length": {"type": "enum", "enum": ["short", "medium", "long"]}
|
||||
},
|
||||
"additionalProperties": false
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
/*app_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
|
||||
let ToolSpec::Function(ResponsesApiTool { description, .. }) =
|
||||
&find_tool(&tools, "mcp__sample__fn").spec
|
||||
else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
|
||||
assert!(description.contains("mcp__sample__fn(args: { open?: Array<{"));
|
||||
assert!(description.contains("lineno?: number | null;"));
|
||||
assert!(description.contains("ref_id: string;"));
|
||||
assert!(description.contains("response_length?: \"short\" | \"medium\" | \"long\";"));
|
||||
assert!(description.contains("tagged_list?: Array<{"));
|
||||
assert!(description.contains("kind: \"tagged\";"));
|
||||
assert!(description.contains("variant: \"alpha\" | \"beta\";"));
|
||||
assert!(!description.contains("open?: string;"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_only_groups_mcp_namespace_instructions_once() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::CodeModeOnly);
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
|
||||
let mut namespace_meta = Meta::new();
|
||||
namespace_meta.insert("codex_namespace".to_string(), json!("mcp__sample"));
|
||||
namespace_meta.insert(
|
||||
"codex_namespace_description".to_string(),
|
||||
json!("Shared namespace guidance."),
|
||||
);
|
||||
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
Some(HashMap::from([
|
||||
(
|
||||
"mcp__sample__alpha".to_string(),
|
||||
mcp_tool_with_meta(
|
||||
"alpha",
|
||||
"",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"id": {"type": "integer"}
|
||||
},
|
||||
"required": ["id"]
|
||||
}),
|
||||
Some(namespace_meta.clone()),
|
||||
),
|
||||
),
|
||||
(
|
||||
"mcp__sample__beta".to_string(),
|
||||
mcp_tool_with_meta(
|
||||
"beta",
|
||||
"",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"q": {"type": "string"}
|
||||
},
|
||||
"required": ["q"]
|
||||
}),
|
||||
Some(namespace_meta),
|
||||
),
|
||||
),
|
||||
])),
|
||||
/*app_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
|
||||
let ToolSpec::Freeform(FreeformTool { description, .. }) =
|
||||
&find_tool(&tools, codex_code_mode::PUBLIC_TOOL_NAME).spec
|
||||
else {
|
||||
panic!("expected freeform exec tool");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
description.matches("## mcp__sample").count(),
|
||||
1,
|
||||
"{description}"
|
||||
);
|
||||
assert_eq!(description.matches("Shared namespace guidance.").count(), 1);
|
||||
assert!(description.contains("### `mcp__sample__alpha` (`mcp__sample__alpha`)"));
|
||||
assert!(description.contains("### `mcp__sample__beta` (`mcp__sample__beta`)"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_augments_builtin_tool_descriptions_with_typed_sample() {
|
||||
let model_info = model_info();
|
||||
@@ -1574,7 +1742,7 @@ fn code_mode_augments_builtin_tool_descriptions_with_typed_sample() {
|
||||
|
||||
assert_eq!(
|
||||
description,
|
||||
"View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within <image ...> tags).\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: { path: string; }): Promise<{ detail: string | null; image_url: string; }>; };\n```"
|
||||
"View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within <image ...> tags).\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{ detail: string | null; image_url: string; }>; };\n```"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1729,6 +1897,15 @@ fn build_specs_with_discoverable_tools<'a>(
|
||||
}
|
||||
|
||||
fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> rmcp::model::Tool {
|
||||
mcp_tool_with_meta(name, description, input_schema, None)
|
||||
}
|
||||
|
||||
fn mcp_tool_with_meta(
|
||||
name: &str,
|
||||
description: &str,
|
||||
input_schema: serde_json::Value,
|
||||
meta: Option<Meta>,
|
||||
) -> rmcp::model::Tool {
|
||||
rmcp::model::Tool {
|
||||
name: name.to_string().into(),
|
||||
title: None,
|
||||
@@ -1738,7 +1915,7 @@ fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> r
|
||||
annotations: None,
|
||||
execution: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
meta,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1842,7 +2019,37 @@ fn strip_descriptions_schema(schema: &mut JsonSchema) {
|
||||
match schema {
|
||||
JsonSchema::Boolean { description }
|
||||
| JsonSchema::String { description }
|
||||
| JsonSchema::Number { description } => {
|
||||
| JsonSchema::Number { description }
|
||||
| JsonSchema::Null { description } => {
|
||||
*description = None;
|
||||
}
|
||||
JsonSchema::Const {
|
||||
description,
|
||||
value: _,
|
||||
schema_type: _,
|
||||
}
|
||||
| JsonSchema::Enum {
|
||||
description,
|
||||
values: _,
|
||||
schema_type: _,
|
||||
} => {
|
||||
*description = None;
|
||||
}
|
||||
JsonSchema::AnyOf {
|
||||
variants,
|
||||
description,
|
||||
}
|
||||
| JsonSchema::OneOf {
|
||||
variants,
|
||||
description,
|
||||
}
|
||||
| JsonSchema::AllOf {
|
||||
variants,
|
||||
description,
|
||||
} => {
|
||||
for variant in variants {
|
||||
strip_descriptions_schema(variant);
|
||||
}
|
||||
*description = None;
|
||||
}
|
||||
JsonSchema::Array { items, description } => {
|
||||
|
||||
Reference in New Issue
Block a user