Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions codex-rs/core/src/function_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use thiserror::Error;
pub enum FunctionCallError {
#[error("{0}")]
RespondToModel(String),
#[error("tool input rewritten by hook")]
UpdatedInput(serde_json::Value),
#[error("LocalShellCall without call_id or id")]
MissingLocalShellCallId,
#[error("Fatal error: {0}")]
Expand Down
46 changes: 39 additions & 7 deletions codex-rs/core/src/mcp_tool_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ pub(crate) async fn handle_mcp_tool_call(
)
.await;

let mut invocation = invocation;
if let Some(decision) = maybe_request_mcp_tool_approval(
&sess,
turn_context,
Expand All @@ -212,7 +213,24 @@ pub(crate) async fn handle_mcp_tool_call(
)
.await
{
let tool_input = invocation
.arguments
.clone()
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new()));
let result = match decision {
McpToolApprovalDecision::AcceptWithUpdatedInput(updated_input) => {
invocation.arguments = Some(updated_input);
return handle_approved_mcp_tool_call(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
metadata.as_ref(),
request_meta,
mcp_app_resource_uri,
)
.await;
}
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptForSession
| McpToolApprovalDecision::AcceptAndRemember => {
Expand Down Expand Up @@ -279,8 +297,7 @@ pub(crate) async fn handle_mcp_tool_call(

return HandledMcpToolCall {
result: CallToolResult::from_result(result),
tool_input: arguments_value
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
tool_input,
};
}

Expand Down Expand Up @@ -937,6 +954,7 @@ async fn maybe_track_codex_app_used(
#[derive(Debug, Clone, PartialEq, Eq)]
enum McpToolApprovalDecision {
Accept,
AcceptWithUpdatedInput(JsonValue),
AcceptForSession,
AcceptAndRemember,
Decline { message: Option<String> },
Expand Down Expand Up @@ -1199,21 +1217,34 @@ async fn maybe_request_mcp_tool_approval(
return Some(McpToolApprovalDecision::Accept);
}

let permission_request_tool_input = invocation
.arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new()));
match run_permission_request_hooks(
sess,
turn_context,
call_id,
PermissionRequestPayload {
tool_name: HookToolName::new(hook_tool_name),
tool_input: invocation
.arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
tool_input: permission_request_tool_input.clone(),
},
)
.await
{
Some(PermissionRequestDecision::Allow) => {
Some(PermissionRequestDecision::Allow {
updated_input: Some(updated_input),
}) => {
if updated_input == permission_request_tool_input {
return Some(McpToolApprovalDecision::Accept);
}
return Some(McpToolApprovalDecision::AcceptWithUpdatedInput(
updated_input,
));
}
Some(PermissionRequestDecision::Allow {
updated_input: None,
}) => {
return Some(McpToolApprovalDecision::Accept);
}
Some(PermissionRequestDecision::Deny { message }) => {
Expand Down Expand Up @@ -1967,6 +1998,7 @@ async fn apply_mcp_tool_approval_decision(
}
}
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptWithUpdatedInput(_)
| McpToolApprovalDecision::Decline { .. }
| McpToolApprovalDecision::Cancel
| McpToolApprovalDecision::BlockedBySafetyMonitor(_) => {}
Expand Down
74 changes: 74 additions & 0 deletions codex-rs/core/src/mcp_tool_call_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2347,6 +2347,80 @@ async fn permission_request_hook_allows_mcp_tool_call() {
);
}

#[tokio::test]
async fn permission_request_hook_can_update_mcp_tool_input() {
let (mut session, turn_context) = make_session_and_context().await;
install_mcp_permission_request_hook(
&mut session,
&turn_context,
"mcp__memory__.*",
&serde_json::json!({
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "allow",
"updatedInput": {
"entities": [{
"name": "Grace",
"entityType": "person"
}]
}
}
}
}),
);
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let invocation = McpInvocation {
server: "memory".to_string(),
tool: "create_entities".to_string(),
arguments: Some(serde_json::json!({
"entities": [{
"name": "Ada",
"entityType": "person"
}]
})),
};
let metadata = McpToolApprovalMetadata {
annotations: Some(annotations(
Some(false),
Some(true),
/*open_world*/ None,
)),
connector_id: None,
connector_name: None,
connector_description: None,
tool_title: Some("Create entities".to_string()),
tool_description: None,
mcp_app_resource_uri: None,
codex_apps_meta: None,
openai_file_input_params: None,
};

let decision = maybe_request_mcp_tool_approval(
&session,
&turn_context,
"call-mcp-hook",
&invocation,
"mcp__memory__create_entities",
Some(&metadata),
AppToolApproval::Auto,
)
.await;

assert_eq!(
decision,
Some(McpToolApprovalDecision::AcceptWithUpdatedInput(
serde_json::json!({
"entities": [{
"name": "Grace",
"entityType": "person"
}]
})
))
);
}

#[tokio::test]
async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
let (mut session, turn_context) = make_session_and_context().await;
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
&tool_ctx,
turn.as_ref(),
AskForApproval::Never,
crate::tools::sandboxing::InitialApprovalState::Evaluate,
)
.await
.expect("probe runtime should succeed");
Expand Down
5 changes: 5 additions & 0 deletions codex-rs/core/src/stream_events_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,11 @@ pub(crate) async fn handle_output_item_done(

output.needs_follow_up = true;
}
Err(FunctionCallError::UpdatedInput(_)) => {
return Err(CodexErr::Fatal(
"updated tool input escaped dispatch".to_string(),
));
}
// A fatal error occurred; surface it back into history.
Err(FunctionCallError::Fatal(message)) => {
return Err(CodexErr::Fatal(message));
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/core/src/tools/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@ impl ToolEmitter {
let result = Err(FunctionCallError::RespondToModel(normalized));
(event, result)
}
Err(ToolError::UpdatedInput(updated_input)) => {
return Err(FunctionCallError::UpdatedInput(updated_input));
}
};
self.emit(ctx, event).await;
result
Expand Down
Loading
Loading