mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Expose explicit dynamic tool namespaces in thread start (#27371)
Stacked on #27365. ## Stack note [#27365](https://github.com/openai/codex/pull/27365) kept `thread/start` unchanged and converted its input in `thread_processor`. This PR updates `thread/start` to accept explicit functions and namespaces directly. Legacy per-tool arrays are still accepted and converted while reading the request. As a result, `thread_processor` can validate and pass the tools through directly, which is why some code added in #27365 is removed here. ## Why `thread/start.dynamicTools` still repeats namespace data on each function even though core now stores explicit namespace groups. The request API should use the same shape so each namespace has one description and one member list. ## What changed - Accept top-level functions and explicit namespace objects in `dynamicTools`. - Continue accepting fully legacy flat arrays, including `exposeToContext`. - Reject arrays that mix legacy and canonical entries. - Reuse the protocol types directly and remove the temporary app-server adapter. - Update validation, docs, the test client, and generated schemas. ## Test plan - `just test -p codex-app-server-protocol` - `just test -p codex-app-server dynamic_tool_call_round_trip_sends_text_content_items_to_model` - `just test -p codex-app-server thread_start_normalizes_legacy_dynamic_tools_into_model_request` - `just test -p codex-app-server thread_start_rejects_mixed_dynamic_tool_formats` - `just test -p codex-app-server thread_start_rejects_hidden_dynamic_tools_without_namespace`
This commit is contained in:
@@ -8,6 +8,9 @@ use codex_app_server_protocol::DynamicToolCallOutputContentItem;
|
||||
use codex_app_server_protocol::DynamicToolCallParams;
|
||||
use codex_app_server_protocol::DynamicToolCallResponse;
|
||||
use codex_app_server_protocol::DynamicToolCallStatus;
|
||||
use codex_app_server_protocol::DynamicToolFunctionSpec;
|
||||
use codex_app_server_protocol::DynamicToolNamespaceSpec;
|
||||
use codex_app_server_protocol::DynamicToolNamespaceTool;
|
||||
use codex_app_server_protocol::DynamicToolSpec;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
@@ -157,80 +160,6 @@ async fn thread_start_normalizes_legacy_dynamic_tools_into_model_request() -> Re
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_keeps_hidden_dynamic_tools_out_of_model_requests() -> Result<()> {
|
||||
let responses = vec![create_final_assistant_message_sse_response("Done")?];
|
||||
let server = create_mock_responses_server_sequence_unchecked(responses).await;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let dynamic_tool = DynamicToolSpec {
|
||||
namespace: Some("codex_app".to_string()),
|
||||
name: "hidden_tool".to_string(),
|
||||
description: "Hidden dynamic tool".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"city": { "type": "string" }
|
||||
},
|
||||
"required": ["city"],
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: true,
|
||||
};
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
dynamic_tools: Some(vec![dynamic_tool.clone()]),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let thread_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
|
||||
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
client_user_message_id: None,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "Hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let turn_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
|
||||
)
|
||||
.await??;
|
||||
let _turn: TurnStartResponse = to_response::<TurnStartResponse>(turn_resp)?;
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let bodies = responses_bodies(&server).await?;
|
||||
assert!(
|
||||
bodies
|
||||
.iter()
|
||||
.all(|body| find_tool(body, &dynamic_tool.name).is_none()),
|
||||
"hidden dynamic tool should not be sent to the model"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_rejects_hidden_dynamic_tools_without_namespace() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
@@ -241,8 +170,7 @@ async fn thread_start_rejects_hidden_dynamic_tools_without_namespace() -> Result
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let dynamic_tool = DynamicToolSpec {
|
||||
namespace: None,
|
||||
let dynamic_tool = DynamicToolSpec::Function(DynamicToolFunctionSpec {
|
||||
name: "hidden_tool".to_string(),
|
||||
description: "Hidden dynamic tool".to_string(),
|
||||
input_schema: json!({
|
||||
@@ -251,7 +179,7 @@ async fn thread_start_rejects_hidden_dynamic_tools_without_namespace() -> Result
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: true,
|
||||
};
|
||||
});
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
@@ -272,7 +200,7 @@ async fn thread_start_rejects_hidden_dynamic_tools_without_namespace() -> Result
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_rejects_dynamic_tools_not_supported_by_responses() -> Result<()> {
|
||||
async fn thread_start_rejects_invalid_dynamic_tool_inputs() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -281,32 +209,109 @@ async fn thread_start_rejects_dynamic_tools_not_supported_by_responses() -> Resu
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let dynamic_tool = DynamicToolSpec {
|
||||
namespace: Some("codex.app".to_string()),
|
||||
name: "lookup.ticket".to_string(),
|
||||
description: "Invalid dynamic tool".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: false,
|
||||
};
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
dynamic_tools: Some(vec![dynamic_tool]),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let error = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(thread_req)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(error.error.code, -32600);
|
||||
assert!(error.error.message.contains("Responses API"));
|
||||
assert!(error.error.message.contains("lookup.ticket"));
|
||||
for (dynamic_tools, expected_error) in [
|
||||
(
|
||||
json!([
|
||||
{
|
||||
"type": "function",
|
||||
"name": "canonical_tool",
|
||||
"description": "Canonical tool",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {}
|
||||
}
|
||||
},
|
||||
{
|
||||
"namespace": "legacy_app",
|
||||
"name": "legacy_tool",
|
||||
"description": "Legacy tool",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {}
|
||||
}
|
||||
}
|
||||
]),
|
||||
"either canonical or legacy format",
|
||||
),
|
||||
(
|
||||
json!([{
|
||||
"type": "namespace",
|
||||
"name": "canonical_namespace",
|
||||
"description": "Canonical namespace",
|
||||
"tools": [{
|
||||
"type": "function",
|
||||
"name": "legacy_visibility_tool",
|
||||
"description": "Uses a legacy visibility field",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {}
|
||||
},
|
||||
"exposeToContext": false
|
||||
}]
|
||||
}]),
|
||||
"either canonical or legacy format",
|
||||
),
|
||||
(
|
||||
json!([{
|
||||
"type": "namespace",
|
||||
"name": "empty_namespace",
|
||||
"description": "Contains no tools",
|
||||
"tools": []
|
||||
}]),
|
||||
"must contain at least one tool",
|
||||
),
|
||||
(
|
||||
json!([
|
||||
{
|
||||
"type": "namespace",
|
||||
"name": "duplicate_namespace",
|
||||
"description": "First namespace",
|
||||
"tools": [{
|
||||
"type": "function",
|
||||
"name": "first_tool",
|
||||
"description": "First tool",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {}
|
||||
}
|
||||
}]
|
||||
},
|
||||
{
|
||||
"type": "namespace",
|
||||
"name": "duplicate_namespace",
|
||||
"description": "Second namespace",
|
||||
"tools": [{
|
||||
"type": "function",
|
||||
"name": "second_tool",
|
||||
"description": "Second tool",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {}
|
||||
}
|
||||
}]
|
||||
}
|
||||
]),
|
||||
"duplicate dynamic tool namespace",
|
||||
),
|
||||
] {
|
||||
let thread_req = mcp
|
||||
.send_raw_request(
|
||||
"thread/start",
|
||||
Some(json!({ "dynamicTools": dynamic_tools })),
|
||||
)
|
||||
.await?;
|
||||
let error = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(thread_req)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(error.error.code, -32600);
|
||||
assert!(
|
||||
error.error.message.contains(expected_error),
|
||||
"unexpected error: {}",
|
||||
error.error.message
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -346,20 +351,41 @@ async fn dynamic_tool_call_round_trip_sends_text_content_items_to_model() -> Res
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let dynamic_tool = DynamicToolSpec {
|
||||
namespace: Some(tool_namespace.to_string()),
|
||||
name: tool_name.to_string(),
|
||||
description: "Demo dynamic tool".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"city": { "type": "string" }
|
||||
},
|
||||
"required": ["city"],
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: false,
|
||||
};
|
||||
let input_schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"city": { "type": "string" }
|
||||
},
|
||||
"required": ["city"],
|
||||
"additionalProperties": false,
|
||||
});
|
||||
let status_schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ticket_id": { "type": "string" }
|
||||
},
|
||||
"required": ["ticket_id"],
|
||||
"additionalProperties": false,
|
||||
});
|
||||
let namespace_description = "Demo namespace tools";
|
||||
let dynamic_tool = DynamicToolSpec::Namespace(DynamicToolNamespaceSpec {
|
||||
name: tool_namespace.to_string(),
|
||||
description: namespace_description.to_string(),
|
||||
tools: vec![
|
||||
DynamicToolNamespaceTool::Function(DynamicToolFunctionSpec {
|
||||
name: tool_name.to_string(),
|
||||
description: "Demo dynamic tool".to_string(),
|
||||
input_schema: input_schema.clone(),
|
||||
defer_loading: false,
|
||||
}),
|
||||
DynamicToolNamespaceTool::Function(DynamicToolFunctionSpec {
|
||||
name: "lookup_status".to_string(),
|
||||
description: "Look up ticket status".to_string(),
|
||||
input_schema: status_schema.clone(),
|
||||
defer_loading: false,
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
@@ -488,6 +514,32 @@ async fn dynamic_tool_call_round_trip_sends_text_content_items_to_model() -> Res
|
||||
.await??;
|
||||
|
||||
let bodies = responses_bodies(&server).await?;
|
||||
let namespace = find_tool(&bodies[0], tool_namespace)
|
||||
.context("expected explicit dynamic tool namespace in first request")?;
|
||||
assert_eq!(
|
||||
namespace,
|
||||
&json!({
|
||||
"type": "namespace",
|
||||
"name": tool_namespace,
|
||||
"description": namespace_description,
|
||||
"tools": [
|
||||
{
|
||||
"type": "function",
|
||||
"name": tool_name,
|
||||
"description": "Demo dynamic tool",
|
||||
"strict": false,
|
||||
"parameters": input_schema,
|
||||
},
|
||||
{
|
||||
"type": "function",
|
||||
"name": "lookup_status",
|
||||
"description": "Look up ticket status",
|
||||
"strict": false,
|
||||
"parameters": status_schema,
|
||||
},
|
||||
],
|
||||
})
|
||||
);
|
||||
let payload = bodies
|
||||
.iter()
|
||||
.find_map(|body| function_call_output_payload(body, call_id))
|
||||
@@ -522,8 +574,7 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let dynamic_tool = DynamicToolSpec {
|
||||
namespace: None,
|
||||
let dynamic_tool = DynamicToolSpec::Function(DynamicToolFunctionSpec {
|
||||
name: tool_name.to_string(),
|
||||
description: "Demo dynamic tool".to_string(),
|
||||
input_schema: json!({
|
||||
@@ -535,7 +586,7 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: false,
|
||||
};
|
||||
});
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
|
||||
@@ -5,6 +5,7 @@ use app_test_support::to_response;
|
||||
use codex_app_server_protocol::DynamicToolCallOutputContentItem;
|
||||
use codex_app_server_protocol::DynamicToolCallParams;
|
||||
use codex_app_server_protocol::DynamicToolCallResponse;
|
||||
use codex_app_server_protocol::DynamicToolFunctionSpec;
|
||||
use codex_app_server_protocol::DynamicToolSpec;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
@@ -126,8 +127,7 @@ async fn thread_unsubscribe_during_turn_keeps_turn_running() -> Result<()> {
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
dynamic_tools: Some(vec![DynamicToolSpec {
|
||||
namespace: None,
|
||||
dynamic_tools: Some(vec![DynamicToolSpec::Function(DynamicToolFunctionSpec {
|
||||
name: tool_name.to_string(),
|
||||
description: "Deterministic wait tool".to_string(),
|
||||
input_schema: json!({
|
||||
@@ -136,7 +136,7 @@ async fn thread_unsubscribe_during_turn_keeps_turn_running() -> Result<()> {
|
||||
"additionalProperties": false,
|
||||
}),
|
||||
defer_loading: false,
|
||||
}]),
|
||||
})]),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
Reference in New Issue
Block a user