mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Expose MCP read-only hints in tool call items (#36055)
## What changed - Propagate MCP tool `readOnlyHint` annotations through tool-call start and completion events. - Include the optional hint in persisted thread history and app-server `mcpToolCall` items, preserving compatibility when the annotation is absent. - Document that the hint describes tool capability, not the outcome of a particular invocation. ## Testing - Cover read-only and write-capable tools in live events and persisted rollout data. - Verify the hint survives thread reads and resumes for both in-progress and completed MCP calls. GitOrigin-RevId: dddfe905146075e5137a1094da485b86b99807f0
This commit is contained in:
@@ -4536,6 +4536,7 @@ async fn turn_event_counts_completed_tool_items() {
|
||||
}),
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms,
|
||||
|
||||
@@ -4373,6 +4373,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
+6
@@ -19261,6 +19261,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
+6
@@ -17025,6 +17025,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -900,6 +900,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -900,6 +900,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1045,6 +1045,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1594,6 +1594,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1385,6 +1385,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
+6
@@ -1385,6 +1385,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1385,6 +1385,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1594,6 +1594,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1385,6 +1385,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1594,6 +1594,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1385,6 +1385,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1385,6 +1385,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1045,6 +1045,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1045,6 +1045,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -1045,6 +1045,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"readOnlyHint": {
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"result": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -69,7 +69,7 @@ durationMs: number | null, } | { "type": "fileChange", id: string, changes: Arra
|
||||
/**
|
||||
* Deprecated: use `appContext.resourceUri` instead.
|
||||
*/
|
||||
mcpAppResourceUri?: string, pluginId: string | null, result: McpToolCallResult | null, error: McpToolCallError | null,
|
||||
mcpAppResourceUri?: string, pluginId: string | null, readOnlyHint: boolean | null, result: McpToolCallResult | null, error: McpToolCallError | null,
|
||||
/**
|
||||
* The duration of the MCP tool call in milliseconds.
|
||||
*/
|
||||
|
||||
@@ -787,6 +787,7 @@ impl ThreadHistoryBuilder {
|
||||
}),
|
||||
mcp_app_resource_uri: payload.mcp_app_resource_uri.clone(),
|
||||
plugin_id: payload.plugin_id.clone(),
|
||||
read_only_hint: payload.read_only_hint,
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -839,6 +840,7 @@ impl ThreadHistoryBuilder {
|
||||
}),
|
||||
mcp_app_resource_uri: payload.mcp_app_resource_uri.clone(),
|
||||
plugin_id: payload.plugin_id.clone(),
|
||||
read_only_hint: payload.read_only_hint,
|
||||
result,
|
||||
error,
|
||||
duration_ms,
|
||||
@@ -2782,6 +2784,7 @@ mod tests {
|
||||
app_name: None,
|
||||
action_name: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
duration: Duration::from_millis(8),
|
||||
result: Err("boom".into()),
|
||||
}),
|
||||
@@ -2840,6 +2843,7 @@ mod tests {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: Some(McpToolCallError {
|
||||
message: "boom".into(),
|
||||
@@ -2872,6 +2876,7 @@ mod tests {
|
||||
app_name: Some("Calendar".into()),
|
||||
action_name: Some("lookup".into()),
|
||||
plugin_id: Some("sample@test".into()),
|
||||
read_only_hint: Some(false),
|
||||
duration: Duration::from_millis(8),
|
||||
result: Ok(CallToolResult {
|
||||
content: vec![serde_json::json!({
|
||||
@@ -2910,6 +2915,7 @@ mod tests {
|
||||
}),
|
||||
mcp_app_resource_uri: Some("ui://widget/lookup.html".into()),
|
||||
plugin_id: Some("sample@test".into()),
|
||||
read_only_hint: Some(false),
|
||||
result: Some(Box::new(McpToolCallResult {
|
||||
content: vec![serde_json::json!({
|
||||
"type": "text",
|
||||
|
||||
@@ -317,6 +317,7 @@ pub enum ThreadItem {
|
||||
/// Deprecated: use `appContext.resourceUri` instead.
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
plugin_id: Option<String>,
|
||||
read_only_hint: Option<bool>,
|
||||
result: Option<Box<McpToolCallResult>>,
|
||||
error: Option<McpToolCallError>,
|
||||
/// The duration of the MCP tool call in milliseconds.
|
||||
@@ -961,6 +962,7 @@ impl From<CoreTurnItem> for ThreadItem {
|
||||
}),
|
||||
mcp_app_resource_uri: mcp.mcp_app_resource_uri,
|
||||
plugin_id: mcp.plugin_id,
|
||||
read_only_hint: mcp.read_only_hint,
|
||||
result: mcp.result.map(McpToolCallResult::from).map(Box::new),
|
||||
error: mcp.error.map(McpToolCallError::from),
|
||||
duration_ms,
|
||||
|
||||
@@ -3144,6 +3144,7 @@ fn core_turn_item_into_thread_item_converts_supported_variants() {
|
||||
app_name: Some("Calendar".to_string()),
|
||||
action_name: Some("create_event".to_string()),
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
read_only_hint: Some(true),
|
||||
status: CoreMcpToolCallStatus::InProgress,
|
||||
result: None,
|
||||
error: None,
|
||||
@@ -3167,6 +3168,7 @@ fn core_turn_item_into_thread_item_converts_supported_variants() {
|
||||
}),
|
||||
mcp_app_resource_uri: Some("app://connector".to_string()),
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
read_only_hint: Some(true),
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -3184,6 +3186,7 @@ fn core_turn_item_into_thread_item_converts_supported_variants() {
|
||||
app_name: None,
|
||||
action_name: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(false),
|
||||
status: CoreMcpToolCallStatus::Completed,
|
||||
result: Some(CallToolResult {
|
||||
content: vec![json!({"type": "text", "text": "ok"})],
|
||||
@@ -3206,6 +3209,7 @@ fn core_turn_item_into_thread_item_converts_supported_variants() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(false),
|
||||
result: Some(Box::new(McpToolCallResult {
|
||||
content: vec![json!({"type": "text", "text": "ok"})],
|
||||
structured_content: Some(json!({"ok": true})),
|
||||
@@ -3234,6 +3238,7 @@ fn mcp_tool_call_app_context_serializes_connector_id() {
|
||||
}),
|
||||
mcp_app_resource_uri: Some("app://connector".to_string()),
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(false),
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -3257,6 +3262,7 @@ fn mcp_tool_call_app_context_serializes_connector_id() {
|
||||
},
|
||||
"mcpAppResourceUri": "app://connector",
|
||||
"pluginId": null,
|
||||
"readOnlyHint": false,
|
||||
"result": null,
|
||||
"error": null,
|
||||
"durationMs": null,
|
||||
|
||||
@@ -1503,7 +1503,7 @@ The app-server streams JSON-RPC notifications while a turn is running. Each turn
|
||||
- `reasoning` — `{id, summary, content}` where `summary` holds streamed reasoning summaries (applicable for most OpenAI models) and `content` holds raw reasoning blocks (applicable for e.g. open source models).
|
||||
- `commandExecution` — `{id, pluginId?, scriptPath?, command, cwd, status, commandActions, aggregatedOutput?, exitCode?, durationMs?}` for sandboxed commands; `pluginId` is present only for commands attributed to a trusted first-party plugin, newly attributed items also include `scriptPath` as a safe `/`-separated path relative to the trusted plugin root, older history may omit `scriptPath`, and `status` is `inProgress`, `completed`, `failed`, or `declined`.
|
||||
- `fileChange` — `{id, changes, status}` describing proposed edits; `changes` list `{path, kind, diff}` and `status` is `inProgress`, `completed`, `failed`, or `declined`.
|
||||
- `mcpToolCall` — `{id, server, tool, status, arguments, appContext, mcpAppResourceUri?, pluginId, result?, error?}` describing MCP calls; `appContext` is `{connectorId, linkId, resourceUri, appName, actionName}` for calls through a trusted MCP app, where `connectorId` identifies the connector that owns the tool, `linkId` identifies the app link, `resourceUri` points to the widget template, `appName` is the connector's display name, and `actionName` is the stable connector `Action.name`. `appName` and `actionName` may be null for older rollout entries. The top-level `mcpAppResourceUri` is deprecated and temporarily duplicated for client migration. `tool` identifies the raw MCP tool. `status` is `inProgress`, `completed`, or `failed`.
|
||||
- `mcpToolCall` — `{id, server, tool, status, arguments, appContext, mcpAppResourceUri?, pluginId, readOnlyHint, result?, error?}` describing MCP calls; `appContext` is `{connectorId, linkId, resourceUri, appName, actionName}` for calls through a trusted MCP app, where `connectorId` identifies the connector that owns the tool, `linkId` identifies the app link, `resourceUri` points to the widget template, `appName` is the connector's display name, and `actionName` is the stable connector `Action.name`. `readOnlyHint` is `true` for read-only tools, `false` for write-capable tools, and `null` when the annotation is unavailable, including older rollout entries. The hint describes tool capability, not whether an invocation succeeded or performed a write; use `status`, `result`, and `error` to determine the execution outcome. `appName` and `actionName` may be null for older rollout entries. The top-level `mcpAppResourceUri` is deprecated and temporarily duplicated for client migration. `tool` identifies the raw MCP tool. `status` is `inProgress`, `completed`, or `failed`.
|
||||
- `collabToolCall` — `{id, tool, status, senderThreadId, receiverThreadId?, newThreadId?, prompt?, agentStatus?}` describing collab tool calls (`spawn_agent`, `send_input`, `resume_agent`, `wait`, `close_agent`); `status` is `inProgress`, `completed`, or `failed`.
|
||||
- `webSearch` — `{id, query, action?, results?}` for a web search request issued by the agent; `action` mirrors the Responses API web_search action payload (`search`, `open_page`, `find_in_page`) and may be omitted until completion. For standalone web search, `results` contains the out-of-band structured result DTOs returned by `/v1/alpha/search`; clients should ignore result types and fields they do not understand.
|
||||
- `imageView` — `{id, path}` emitted when the agent invokes the image viewer tool.
|
||||
|
||||
@@ -89,6 +89,7 @@ mod tests {
|
||||
}),
|
||||
mcp_app_resource_uri: Some("ui://widget/lookup.html".to_string()),
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
read_only_hint: None,
|
||||
result: Some(Box::new(McpToolCallResult {
|
||||
content: vec![serde_json::json!({
|
||||
"type": "text",
|
||||
@@ -138,6 +139,7 @@ mod tests {
|
||||
}),
|
||||
mcp_app_resource_uri: Some("ui://widget/lookup.html".to_string()),
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
read_only_hint: None,
|
||||
result: Some(Box::new(redacted_mcp_tool_call_result())),
|
||||
error: None,
|
||||
duration_ms: Some(8),
|
||||
@@ -156,6 +158,7 @@ mod tests {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: Some(McpToolCallError {
|
||||
message: "secret error".to_string(),
|
||||
@@ -176,6 +179,7 @@ mod tests {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: Some(McpToolCallError {
|
||||
message: REDACTED_PAYLOAD.to_string(),
|
||||
|
||||
@@ -25,6 +25,10 @@ use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SelectedCapabilityRoot;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadReadParams;
|
||||
use codex_app_server_protocol::ThreadReadResponse;
|
||||
use codex_app_server_protocol::ThreadResumeParams;
|
||||
use codex_app_server_protocol::ThreadResumeResponse;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::TurnEnvironmentParams;
|
||||
@@ -561,6 +565,7 @@ async fn mcp_tool_call_completion_notification_contains_truncated_large_result()
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: Some(result),
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -579,6 +584,164 @@ async fn mcp_tool_call_completion_notification_contains_truncated_large_result()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn mcp_tool_call_hint_survives_mid_call_thread_read_and_resume() -> Result<()> {
|
||||
let call_id = "call-mid-flight-mcp";
|
||||
let namespace = format!("mcp__{TEST_SERVER_NAME}");
|
||||
let responses = vec![
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-mid-flight"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
call_id,
|
||||
&namespace,
|
||||
TEST_TOOL_NAME,
|
||||
&serde_json::to_string(&json!({
|
||||
"message": ELICITATION_TRIGGER_MESSAGE,
|
||||
}))?,
|
||||
),
|
||||
responses::ev_completed("resp-mid-flight"),
|
||||
]),
|
||||
create_final_assistant_message_sse_response("done")?,
|
||||
];
|
||||
let responses_server = create_mock_responses_server_sequence(responses).await;
|
||||
let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?;
|
||||
let codex_home = TempDir::new()?;
|
||||
mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT)
|
||||
.write(codex_home.path())?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
approval_policy: Some(codex_app_server_protocol::AskForApproval::UnlessTrusted),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let TurnStartResponse { turn, .. } = mcp
|
||||
.request(|request_id| ClientRequest::TurnStart {
|
||||
request_id,
|
||||
params: TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
client_user_message_id: None,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "Call the MCP tool".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
|
||||
let server_request = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_request_message(),
|
||||
)
|
||||
.await??;
|
||||
let ServerRequest::McpServerElicitationRequest { request_id, .. } = server_request else {
|
||||
panic!("expected MCP elicitation while the tool call is in progress");
|
||||
};
|
||||
|
||||
let read_id = mcp
|
||||
.send_thread_read_request(ThreadReadParams {
|
||||
thread_id: thread.id.clone(),
|
||||
include_turns: true,
|
||||
})
|
||||
.await?;
|
||||
let ThreadReadResponse {
|
||||
thread: read_thread,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(read_id)).await??;
|
||||
assert_eq!(read_thread.id, thread.id);
|
||||
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id: thread.id.clone(),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadResumeResponse {
|
||||
thread: resumed_thread,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
let expected_item = ThreadItem::McpToolCall {
|
||||
id: call_id.to_string(),
|
||||
server: TEST_SERVER_NAME.to_string(),
|
||||
tool: TEST_TOOL_NAME.to_string(),
|
||||
status: McpToolCallStatus::InProgress,
|
||||
arguments: json!({ "message": ELICITATION_TRIGGER_MESSAGE }),
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(true),
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
};
|
||||
let resumed_item = resumed_thread
|
||||
.turns
|
||||
.iter()
|
||||
.flat_map(|turn| &turn.items)
|
||||
.find(|item| matches!(item, ThreadItem::McpToolCall { id, .. } if id == call_id))
|
||||
.expect("resumed thread should include the in-progress MCP tool call");
|
||||
assert_eq!(resumed_item, &expected_item);
|
||||
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(McpServerElicitationRequestResponse {
|
||||
action: McpServerElicitationAction::Accept,
|
||||
content: Some(json!({ "confirmed": true })),
|
||||
meta: None,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let completed = wait_for_mcp_tool_call_completed(&mut mcp, call_id).await?;
|
||||
assert_eq!(completed.turn_id, turn.id);
|
||||
let ThreadItem::McpToolCall {
|
||||
status,
|
||||
read_only_hint,
|
||||
..
|
||||
} = &completed.item
|
||||
else {
|
||||
panic!("expected the completed MCP tool call item");
|
||||
};
|
||||
assert_eq!(status, &McpToolCallStatus::Completed);
|
||||
assert_eq!(read_only_hint, &Some(true));
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let completed_read_id = mcp
|
||||
.send_thread_read_request(ThreadReadParams {
|
||||
thread_id: thread.id,
|
||||
include_turns: true,
|
||||
})
|
||||
.await?;
|
||||
let ThreadReadResponse {
|
||||
thread: completed_read,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(completed_read_id)).await??;
|
||||
let persisted_item = completed_read
|
||||
.turns
|
||||
.iter()
|
||||
.flat_map(|turn| &turn.items)
|
||||
.find(|item| matches!(item, ThreadItem::McpToolCall { id, .. } if id == call_id))
|
||||
.expect("completed thread history should include the persisted MCP tool call");
|
||||
assert_eq!(persisted_item, &completed.item);
|
||||
|
||||
mcp_server_handle.abort();
|
||||
let _ = mcp_server_handle.await;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
struct ToolAppsMcpServer;
|
||||
|
||||
|
||||
@@ -1357,6 +1357,7 @@ async fn thread_resume_redacts_payloads_for_chatgpt_remote_clients() -> Result<(
|
||||
let ThreadItem::McpToolCall {
|
||||
arguments,
|
||||
app_context,
|
||||
read_only_hint,
|
||||
result,
|
||||
error,
|
||||
..
|
||||
@@ -1375,6 +1376,7 @@ async fn thread_resume_redacts_payloads_for_chatgpt_remote_clients() -> Result<(
|
||||
action_name: Some("lookup".to_string()),
|
||||
})
|
||||
);
|
||||
assert_eq!(read_only_hint, &Some(false));
|
||||
let result = result.as_ref().expect("redacted MCP result");
|
||||
assert_eq!(
|
||||
result.content,
|
||||
@@ -1410,6 +1412,7 @@ async fn thread_resume_redacts_payloads_for_chatgpt_remote_clients() -> Result<(
|
||||
let ThreadItem::McpToolCall {
|
||||
arguments,
|
||||
app_context,
|
||||
read_only_hint,
|
||||
result,
|
||||
..
|
||||
} = normal_mcp_item
|
||||
@@ -1427,6 +1430,7 @@ async fn thread_resume_redacts_payloads_for_chatgpt_remote_clients() -> Result<(
|
||||
action_name: Some("lookup".to_string()),
|
||||
})
|
||||
);
|
||||
assert_eq!(read_only_hint, &Some(false));
|
||||
let result = result.as_ref().expect("normal MCP result");
|
||||
assert_eq!(
|
||||
result.content,
|
||||
@@ -1534,6 +1538,7 @@ fn append_resume_redaction_history(
|
||||
app_name: Some("Calendar".to_string()),
|
||||
action_name: Some("lookup".to_string()),
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(false),
|
||||
duration: Duration::from_millis(8),
|
||||
result: Ok(CallToolResult {
|
||||
content: vec![json!({
|
||||
|
||||
@@ -340,6 +340,7 @@ struct McpToolCallItemMetadata {
|
||||
app_name: Option<String>,
|
||||
action_name: Option<String>,
|
||||
plugin_id: Option<String>,
|
||||
read_only_hint: Option<bool>,
|
||||
}
|
||||
|
||||
impl McpToolCallItemMetadata {
|
||||
@@ -364,6 +365,9 @@ impl McpToolCallItemMetadata {
|
||||
.filter(|action_name| !action_name.is_empty())
|
||||
.map(str::to_string),
|
||||
plugin_id: metadata.and_then(|metadata| metadata.plugin_id.clone()),
|
||||
read_only_hint: metadata
|
||||
.and_then(|metadata| metadata.annotations.as_ref())
|
||||
.and_then(|annotations| annotations.read_only_hint),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -918,6 +922,7 @@ async fn notify_mcp_tool_call_started(
|
||||
app_name: item_metadata.app_name,
|
||||
action_name: item_metadata.action_name,
|
||||
plugin_id: item_metadata.plugin_id,
|
||||
read_only_hint: item_metadata.read_only_hint,
|
||||
status: McpToolCallStatus::InProgress,
|
||||
result: None,
|
||||
error: None,
|
||||
@@ -962,6 +967,7 @@ async fn notify_mcp_tool_call_completed(
|
||||
app_name: item_metadata.app_name,
|
||||
action_name: item_metadata.action_name,
|
||||
plugin_id: item_metadata.plugin_id,
|
||||
read_only_hint: item_metadata.read_only_hint,
|
||||
status,
|
||||
result,
|
||||
error,
|
||||
|
||||
@@ -1155,6 +1155,11 @@ fn mcp_tool_call_item_metadata_only_trusts_codex_apps_identity() {
|
||||
/*tool_description*/ None,
|
||||
);
|
||||
metadata.link_id = Some("link_fedcba9876543210fedcba9876543210".to_string());
|
||||
metadata.annotations = Some(annotations(
|
||||
Some(false),
|
||||
/*destructive*/ None,
|
||||
/*open_world*/ None,
|
||||
));
|
||||
metadata.codex_apps_meta = Some(
|
||||
serde_json::json!({
|
||||
"resource_uri": "/asdk_app_0123456789abcdef0123456789abcdef/link_fedcba9876543210fedcba9876543210/create_event",
|
||||
@@ -1173,6 +1178,7 @@ fn mcp_tool_call_item_metadata_only_trusts_codex_apps_identity() {
|
||||
app_name: Some("Calendar".to_string()),
|
||||
action_name: Some("create_event".to_string()),
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(false),
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -1184,6 +1190,7 @@ fn mcp_tool_call_item_metadata_only_trusts_codex_apps_identity() {
|
||||
app_name: None,
|
||||
action_name: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(false),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -1208,6 +1215,7 @@ async fn mcp_tool_call_item_includes_app_identity() {
|
||||
app_name: Some("Calendar".to_string()),
|
||||
action_name: Some("create_event".to_string()),
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
read_only_hint: Some(false),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -1234,6 +1242,7 @@ async fn mcp_tool_call_item_includes_app_identity() {
|
||||
assert_eq!(item.plugin_id.as_deref(), Some("sample@test"));
|
||||
assert_eq!(item.app_name.as_deref(), Some("Calendar"));
|
||||
assert_eq!(item.action_name.as_deref(), Some("create_event"));
|
||||
assert_eq!(item.read_only_hint, Some(false));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -227,6 +227,7 @@ async fn emit_tool_call_begin(
|
||||
app_name: None,
|
||||
action_name: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
status: McpToolCallStatus::InProgress,
|
||||
result: None,
|
||||
error: None,
|
||||
@@ -270,6 +271,7 @@ async fn emit_tool_call_end(
|
||||
app_name: None,
|
||||
action_name: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
status,
|
||||
result,
|
||||
error,
|
||||
|
||||
@@ -9,11 +9,15 @@ use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::items::McpToolCallStatus;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ElicitationAction;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use codex_protocol::request_user_input::RequestUserInputAnswer;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -129,6 +133,25 @@ async fn submit_user_turn(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn wait_for_mcp_tool_call_item(
|
||||
test: &TestCodex,
|
||||
call_id: &str,
|
||||
status: McpToolCallStatus,
|
||||
) -> Option<bool> {
|
||||
wait_for_event_match(&test.codex, |event| {
|
||||
let item = match event {
|
||||
EventMsg::ItemStarted(event) => &event.item,
|
||||
EventMsg::ItemCompleted(event) => &event.item,
|
||||
_ => return None,
|
||||
};
|
||||
let TurnItem::McpToolCall(item) = item else {
|
||||
return None;
|
||||
};
|
||||
(item.id == call_id && item.status == status).then_some(item.read_only_hint)
|
||||
})
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -405,6 +428,10 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
wait_for_mcp_tool_call_item(&test, read_call_id, McpToolCallStatus::InProgress).await,
|
||||
Some(true)
|
||||
);
|
||||
let EventMsg::McpToolCallBegin(read_begin) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::McpToolCallBegin(_))
|
||||
})
|
||||
@@ -413,7 +440,16 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
|
||||
unreachable!("event guard guarantees McpToolCallBegin");
|
||||
};
|
||||
assert_eq!(read_begin.call_id, read_call_id);
|
||||
assert_eq!(read_begin.read_only_hint, Some(true));
|
||||
|
||||
assert_eq!(
|
||||
wait_for_mcp_tool_call_item(&test, read_call_id, McpToolCallStatus::Completed).await,
|
||||
Some(true)
|
||||
);
|
||||
assert_eq!(
|
||||
wait_for_mcp_tool_call_item(&test, write_call_id, McpToolCallStatus::InProgress).await,
|
||||
Some(false)
|
||||
);
|
||||
let next_route = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
@@ -425,6 +461,7 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
|
||||
panic!("read-only app action should not prompt in writes mode");
|
||||
};
|
||||
assert_eq!(write_begin.call_id, write_call_id);
|
||||
assert_eq!(write_begin.read_only_hint, Some(false));
|
||||
|
||||
let EventMsg::ElicitationRequest(request) = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
@@ -447,6 +484,10 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
|
||||
})
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
wait_for_mcp_tool_call_item(&test, write_call_id, McpToolCallStatus::Completed).await,
|
||||
Some(false)
|
||||
);
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
@@ -456,6 +497,32 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
|
||||
recorded_apps_tool_call_by_call_id(&server, read_call_id).await;
|
||||
recorded_apps_tool_call_by_call_id(&server, write_call_id).await;
|
||||
|
||||
test.codex.ensure_rollout_materialized().await;
|
||||
test.codex.flush_rollout().await?;
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let persisted_hints = tokio::fs::read_to_string(rollout_path)
|
||||
.await?
|
||||
.lines()
|
||||
.map(serde_json::from_str::<RolloutLine>)
|
||||
.collect::<serde_json::Result<Vec<_>>>()?
|
||||
.into_iter()
|
||||
.filter_map(|line| match line.item {
|
||||
RolloutItem::EventMsg(EventMsg::McpToolCallEnd(event))
|
||||
if event.call_id == read_call_id || event.call_id == write_call_id =>
|
||||
{
|
||||
Some((event.call_id, event.read_only_hint))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
persisted_hints,
|
||||
vec![
|
||||
(read_call_id.to_string(), Some(true)),
|
||||
(write_call_id.to_string(), Some(false)),
|
||||
]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -1970,6 +1970,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
|
||||
app_name: None,
|
||||
action_name: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: Some(true),
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -101,6 +101,7 @@ fn mcp_tool_call_result_preserves_meta_in_jsonl_event() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: Some(Box::new(codex_app_server_protocol::McpToolCallResult {
|
||||
content: vec![json!({"type": "text", "text": "search result"})],
|
||||
structured_content: None,
|
||||
|
||||
@@ -489,6 +489,7 @@ fn mcp_tool_call_begin_and_end_emit_item_events() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -508,6 +509,7 @@ fn mcp_tool_call_begin_and_end_emit_item_events() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: Some(Box::new(McpToolCallResult {
|
||||
content: Vec::new(),
|
||||
structured_content: None,
|
||||
@@ -581,6 +583,7 @@ fn mcp_tool_call_failure_sets_failed_status() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: Some(McpToolCallError {
|
||||
message: "tool exploded".to_string(),
|
||||
@@ -631,6 +634,7 @@ fn mcp_tool_call_defaults_arguments_and_preserves_structured_content() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -650,6 +654,7 @@ fn mcp_tool_call_defaults_arguments_and_preserves_structured_content() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: Some(Box::new(McpToolCallResult {
|
||||
content: vec![json!({
|
||||
"type": "text",
|
||||
|
||||
@@ -400,6 +400,9 @@ pub struct McpToolCallItem {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub plugin_id: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub read_only_hint: Option<bool>,
|
||||
pub status: McpToolCallStatus,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
|
||||
@@ -473,6 +473,7 @@ impl McpToolCallItem {
|
||||
app_name: self.app_name.clone(),
|
||||
action_name: self.action_name.clone(),
|
||||
plugin_id: self.plugin_id.clone(),
|
||||
read_only_hint: self.read_only_hint,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -496,6 +497,7 @@ impl McpToolCallItem {
|
||||
app_name: self.app_name.clone(),
|
||||
action_name: self.action_name.clone(),
|
||||
plugin_id: self.plugin_id.clone(),
|
||||
read_only_hint: self.read_only_hint,
|
||||
duration: self.duration?,
|
||||
result,
|
||||
}))
|
||||
|
||||
@@ -2430,6 +2430,10 @@ pub struct McpToolCallBeginEvent {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub plugin_id: Option<String>,
|
||||
/// Whether the selected tool is annotated as read-only, not its execution outcome.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub read_only_hint: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq)]
|
||||
@@ -2455,6 +2459,9 @@ pub struct McpToolCallEndEvent {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub plugin_id: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub read_only_hint: Option<bool>,
|
||||
#[ts(type = "string")]
|
||||
pub duration: Duration,
|
||||
/// Result of the tool call. Note this could be an error.
|
||||
@@ -5308,6 +5315,7 @@ mod tests {
|
||||
app_name: Some("Calendar".into()),
|
||||
action_name: Some("create_event".into()),
|
||||
plugin_id: Some("sample@test".into()),
|
||||
read_only_hint: Some(false),
|
||||
status: McpToolCallStatus::InProgress,
|
||||
result: None,
|
||||
error: None,
|
||||
@@ -5331,6 +5339,7 @@ mod tests {
|
||||
assert_eq!(event.app_name.as_deref(), Some("Calendar"));
|
||||
assert_eq!(event.action_name.as_deref(), Some("create_event"));
|
||||
assert_eq!(event.plugin_id.as_deref(), Some("sample@test"));
|
||||
assert_eq!(event.read_only_hint, Some(false));
|
||||
}
|
||||
_ => panic!("expected McpToolCallBegin event"),
|
||||
}
|
||||
@@ -5426,6 +5435,7 @@ mod tests {
|
||||
app_name: Some("Calendar".into()),
|
||||
action_name: Some("create_event".into()),
|
||||
plugin_id: Some("sample@test".into()),
|
||||
read_only_hint: None,
|
||||
status: McpToolCallStatus::Completed,
|
||||
result: Some(CallToolResult {
|
||||
content: vec![json!({"type": "text", "text": "ok"})],
|
||||
|
||||
@@ -1171,6 +1171,7 @@ async fn replayed_in_progress_mcp_tool_call_stays_active() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -1204,6 +1205,7 @@ async fn deferred_mcp_lifecycle_events_keep_fifo_after_stream_finishes() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: None,
|
||||
error: None,
|
||||
duration_ms: None,
|
||||
@@ -1220,6 +1222,7 @@ async fn deferred_mcp_lifecycle_events_keep_fifo_after_stream_finishes() {
|
||||
app_context: None,
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
read_only_hint: None,
|
||||
result: Some(Box::new(codex_app_server_protocol::McpToolCallResult {
|
||||
content: vec![json!({"type": "text", "text": "deferred result"})],
|
||||
structured_content: None,
|
||||
|
||||
Reference in New Issue
Block a user