mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[elicitations] Support always allow option for mcp tool calls. (#13807)
- [x] Support always allow option for mcp tool calls, writes to config.toml. - [x] Fix config hot-reload after starting a new thread for TUI.
This commit is contained in:
committed by
GitHub
Unverified
parent
590cfa6176
commit
a4a9536fd7
@@ -13,6 +13,8 @@ use crate::analytics_client::InvocationType;
|
||||
use crate::analytics_client::build_track_events_context;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::config::edit::ConfigEdit;
|
||||
use crate::config::edit::ConfigEditsBuilder;
|
||||
use crate::config::types::AppToolApproval;
|
||||
use crate::connectors;
|
||||
use crate::features::Feature;
|
||||
@@ -42,7 +44,9 @@ use codex_rmcp_client::ElicitationAction;
|
||||
use codex_rmcp_client::ElicitationResponse;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use serde::Serialize;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use toml_edit::value;
|
||||
|
||||
/// Handles the specified tool call dispatches the appropriate
|
||||
/// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`.
|
||||
@@ -127,7 +131,9 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
.await
|
||||
{
|
||||
let result = match decision {
|
||||
McpToolApprovalDecision::Accept | McpToolApprovalDecision::AcceptAndRemember => {
|
||||
McpToolApprovalDecision::Accept
|
||||
| McpToolApprovalDecision::AcceptForSession
|
||||
| McpToolApprovalDecision::AcceptAndRemember => {
|
||||
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
|
||||
call_id: call_id.clone(),
|
||||
invocation: invocation.clone(),
|
||||
@@ -352,6 +358,7 @@ async fn maybe_track_codex_app_used(
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum McpToolApprovalDecision {
|
||||
Accept,
|
||||
AcceptForSession,
|
||||
AcceptAndRemember,
|
||||
Decline,
|
||||
Cancel,
|
||||
@@ -366,15 +373,22 @@ struct McpToolApprovalMetadata {
|
||||
tool_description: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct McpToolApprovalPromptOptions {
|
||||
allow_session_remember: bool,
|
||||
allow_persistent_approval: bool,
|
||||
}
|
||||
|
||||
const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval";
|
||||
const MCP_TOOL_APPROVAL_ACCEPT: &str = "Approve Once";
|
||||
const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Approve this Session";
|
||||
const MCP_TOOL_APPROVAL_DECLINE: &str = "Deny";
|
||||
const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Approve this session";
|
||||
const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Always allow";
|
||||
const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel";
|
||||
const MCP_TOOL_APPROVAL_KIND_KEY: &str = "codex_approval_kind";
|
||||
const MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call";
|
||||
const MCP_TOOL_APPROVAL_PERSIST_KEY: &str = "persist";
|
||||
const MCP_TOOL_APPROVAL_PERSIST_SESSION: &str = "session";
|
||||
const MCP_TOOL_APPROVAL_PERSIST_ALWAYS: &str = "always";
|
||||
const MCP_TOOL_APPROVAL_SOURCE_KEY: &str = "source";
|
||||
const MCP_TOOL_APPROVAL_SOURCE_CONNECTOR: &str = "connector";
|
||||
const MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: &str = "connector_id";
|
||||
@@ -384,13 +398,25 @@ const MCP_TOOL_APPROVAL_TOOL_TITLE_KEY: &str = "tool_title";
|
||||
const MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY: &str = "tool_description";
|
||||
const MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: &str = "tool_params";
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
|
||||
struct McpToolApprovalKey {
|
||||
server: String,
|
||||
connector_id: Option<String>,
|
||||
tool_name: String,
|
||||
}
|
||||
|
||||
fn mcp_tool_approval_prompt_options(
|
||||
session_approval_key: Option<&McpToolApprovalKey>,
|
||||
persistent_approval_key: Option<&McpToolApprovalKey>,
|
||||
tool_call_mcp_elicitation_enabled: bool,
|
||||
) -> McpToolApprovalPromptOptions {
|
||||
McpToolApprovalPromptOptions {
|
||||
allow_session_remember: session_approval_key.is_some(),
|
||||
allow_persistent_approval: tool_call_mcp_elicitation_enabled
|
||||
&& persistent_approval_key.is_some(),
|
||||
}
|
||||
}
|
||||
|
||||
async fn maybe_request_mcp_tool_approval(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
@@ -412,25 +438,18 @@ async fn maybe_request_mcp_tool_approval(
|
||||
}
|
||||
}
|
||||
|
||||
let approval_key = if approval_mode == AppToolApproval::Auto {
|
||||
let connector_id = metadata.and_then(|metadata| metadata.connector_id.clone());
|
||||
if invocation.server == CODEX_APPS_MCP_SERVER_NAME && connector_id.is_none() {
|
||||
None
|
||||
} else {
|
||||
Some(McpToolApprovalKey {
|
||||
server: invocation.server.clone(),
|
||||
connector_id,
|
||||
tool_name: invocation.tool.clone(),
|
||||
})
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if let Some(key) = approval_key.as_ref()
|
||||
let session_approval_key = session_mcp_tool_approval_key(invocation, metadata, approval_mode);
|
||||
let persistent_approval_key =
|
||||
persistent_mcp_tool_approval_key(invocation, metadata, approval_mode);
|
||||
if let Some(key) = session_approval_key.as_ref()
|
||||
&& mcp_tool_approval_is_remembered(sess, key).await
|
||||
{
|
||||
return Some(McpToolApprovalDecision::Accept);
|
||||
}
|
||||
let tool_call_mcp_elicitation_enabled = turn_context
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::ToolCallMcpElicitation);
|
||||
|
||||
if routes_approval_to_guardian(turn_context) {
|
||||
let decision = review_approval_request(
|
||||
@@ -440,9 +459,23 @@ async fn maybe_request_mcp_tool_approval(
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
return Some(mcp_tool_approval_decision_from_guardian(decision));
|
||||
let decision = mcp_tool_approval_decision_from_guardian(decision);
|
||||
apply_mcp_tool_approval_decision(
|
||||
sess,
|
||||
turn_context,
|
||||
decision,
|
||||
session_approval_key,
|
||||
persistent_approval_key,
|
||||
)
|
||||
.await;
|
||||
return Some(decision);
|
||||
}
|
||||
|
||||
let prompt_options = mcp_tool_approval_prompt_options(
|
||||
session_approval_key.as_ref(),
|
||||
persistent_approval_key.as_ref(),
|
||||
tool_call_mcp_elicitation_enabled,
|
||||
);
|
||||
let question_id = format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}");
|
||||
let question = build_mcp_tool_approval_question(
|
||||
question_id.clone(),
|
||||
@@ -451,13 +484,9 @@ async fn maybe_request_mcp_tool_approval(
|
||||
metadata.and_then(|metadata| metadata.tool_title.as_deref()),
|
||||
metadata.and_then(|metadata| metadata.connector_name.as_deref()),
|
||||
annotations,
|
||||
approval_key.is_some(),
|
||||
prompt_options,
|
||||
);
|
||||
if turn_context
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::ToolCallMcpElicitation)
|
||||
{
|
||||
if tool_call_mcp_elicitation_enabled {
|
||||
let request_id = rmcp::model::RequestId::String(
|
||||
format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}").into(),
|
||||
);
|
||||
@@ -468,7 +497,7 @@ async fn maybe_request_mcp_tool_approval(
|
||||
metadata,
|
||||
invocation.arguments.as_ref(),
|
||||
question.clone(),
|
||||
approval_key.is_some(),
|
||||
prompt_options,
|
||||
);
|
||||
let decision = parse_mcp_tool_approval_elicitation_response(
|
||||
sess.request_mcp_server_elicitation(turn_context.as_ref(), request_id, params)
|
||||
@@ -476,11 +505,14 @@ async fn maybe_request_mcp_tool_approval(
|
||||
&question_id,
|
||||
);
|
||||
let decision = normalize_approval_decision_for_mode(decision, approval_mode);
|
||||
if matches!(decision, McpToolApprovalDecision::AcceptAndRemember)
|
||||
&& let Some(key) = approval_key
|
||||
{
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
}
|
||||
apply_mcp_tool_approval_decision(
|
||||
sess,
|
||||
turn_context,
|
||||
decision,
|
||||
session_approval_key,
|
||||
persistent_approval_key,
|
||||
)
|
||||
.await;
|
||||
return Some(decision);
|
||||
}
|
||||
|
||||
@@ -494,14 +526,51 @@ async fn maybe_request_mcp_tool_approval(
|
||||
parse_mcp_tool_approval_response(response, &question_id),
|
||||
approval_mode,
|
||||
);
|
||||
if matches!(decision, McpToolApprovalDecision::AcceptAndRemember)
|
||||
&& let Some(key) = approval_key
|
||||
{
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
}
|
||||
apply_mcp_tool_approval_decision(
|
||||
sess,
|
||||
turn_context,
|
||||
decision,
|
||||
session_approval_key,
|
||||
persistent_approval_key,
|
||||
)
|
||||
.await;
|
||||
Some(decision)
|
||||
}
|
||||
|
||||
fn session_mcp_tool_approval_key(
|
||||
invocation: &McpInvocation,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> Option<McpToolApprovalKey> {
|
||||
if approval_mode != AppToolApproval::Auto {
|
||||
return None;
|
||||
}
|
||||
|
||||
let connector_id = metadata.and_then(|metadata| metadata.connector_id.clone());
|
||||
if invocation.server == CODEX_APPS_MCP_SERVER_NAME && connector_id.is_none() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(McpToolApprovalKey {
|
||||
server: invocation.server.clone(),
|
||||
connector_id,
|
||||
tool_name: invocation.tool.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
fn persistent_mcp_tool_approval_key(
|
||||
invocation: &McpInvocation,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> Option<McpToolApprovalKey> {
|
||||
if invocation.server != CODEX_APPS_MCP_SERVER_NAME {
|
||||
return None;
|
||||
}
|
||||
|
||||
session_mcp_tool_approval_key(invocation, metadata, approval_mode)
|
||||
.filter(|key| key.connector_id.is_some())
|
||||
}
|
||||
|
||||
fn build_guardian_mcp_tool_review_request(
|
||||
invocation: &McpInvocation,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
@@ -594,8 +663,8 @@ fn mcp_tool_approval_decision_from_guardian(decision: ReviewDecision) -> McpTool
|
||||
match decision {
|
||||
ReviewDecision::Approved
|
||||
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
|
||||
| ReviewDecision::ApprovedForSession
|
||||
| ReviewDecision::NetworkPolicyAmendment { .. } => McpToolApprovalDecision::Accept,
|
||||
ReviewDecision::ApprovedForSession => McpToolApprovalDecision::AcceptForSession,
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => McpToolApprovalDecision::Decline,
|
||||
}
|
||||
}
|
||||
@@ -691,7 +760,7 @@ fn build_mcp_tool_approval_question(
|
||||
tool_title: Option<&str>,
|
||||
connector_name: Option<&str>,
|
||||
annotations: Option<&ToolAnnotations>,
|
||||
allow_remember_option: bool,
|
||||
prompt_options: McpToolApprovalPromptOptions,
|
||||
) -> RequestUserInputQuestion {
|
||||
let destructive =
|
||||
annotations.and_then(|annotations| annotations.destructive_hint) == Some(true);
|
||||
@@ -721,22 +790,22 @@ fn build_mcp_tool_approval_question(
|
||||
label: MCP_TOOL_APPROVAL_ACCEPT.to_string(),
|
||||
description: "Run the tool and continue.".to_string(),
|
||||
}];
|
||||
if allow_remember_option {
|
||||
if prompt_options.allow_session_remember {
|
||||
options.push(RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER.to_string(),
|
||||
label: MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(),
|
||||
description: "Run the tool and remember this choice for this session.".to_string(),
|
||||
});
|
||||
}
|
||||
options.extend([
|
||||
RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_DECLINE.to_string(),
|
||||
description: "Decline this tool call and continue.".to_string(),
|
||||
},
|
||||
RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_CANCEL.to_string(),
|
||||
description: "Cancel this tool call".to_string(),
|
||||
},
|
||||
]);
|
||||
if prompt_options.allow_persistent_approval {
|
||||
options.push(RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER.to_string(),
|
||||
description: "Run the tool and remember this choice for future tool calls.".to_string(),
|
||||
});
|
||||
}
|
||||
options.push(RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_CANCEL.to_string(),
|
||||
description: "Cancel this tool call".to_string(),
|
||||
});
|
||||
|
||||
RequestUserInputQuestion {
|
||||
id: question_id,
|
||||
@@ -755,7 +824,7 @@ fn build_mcp_tool_approval_elicitation_request(
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
tool_params: Option<&serde_json::Value>,
|
||||
question: RequestUserInputQuestion,
|
||||
allow_session_persist: bool,
|
||||
prompt_options: McpToolApprovalPromptOptions,
|
||||
) -> McpServerElicitationRequestParams {
|
||||
let message = if question.header.trim().is_empty() {
|
||||
question.question
|
||||
@@ -774,7 +843,7 @@ fn build_mcp_tool_approval_elicitation_request(
|
||||
server,
|
||||
metadata,
|
||||
tool_params,
|
||||
allow_session_persist,
|
||||
prompt_options,
|
||||
),
|
||||
message,
|
||||
requested_schema: McpElicitationSchema {
|
||||
@@ -791,18 +860,39 @@ fn build_mcp_tool_approval_elicitation_meta(
|
||||
server: &str,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
tool_params: Option<&serde_json::Value>,
|
||||
allow_session_persist: bool,
|
||||
prompt_options: McpToolApprovalPromptOptions,
|
||||
) -> Option<serde_json::Value> {
|
||||
let mut meta = serde_json::Map::new();
|
||||
meta.insert(
|
||||
MCP_TOOL_APPROVAL_KIND_KEY.to_string(),
|
||||
serde_json::Value::String(MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL.to_string()),
|
||||
);
|
||||
if allow_session_persist {
|
||||
meta.insert(
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(),
|
||||
serde_json::Value::String(MCP_TOOL_APPROVAL_PERSIST_SESSION.to_string()),
|
||||
);
|
||||
match (
|
||||
prompt_options.allow_session_remember,
|
||||
prompt_options.allow_persistent_approval,
|
||||
) {
|
||||
(true, true) => {
|
||||
meta.insert(
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(),
|
||||
serde_json::json!([
|
||||
MCP_TOOL_APPROVAL_PERSIST_SESSION,
|
||||
MCP_TOOL_APPROVAL_PERSIST_ALWAYS,
|
||||
]),
|
||||
);
|
||||
}
|
||||
(true, false) => {
|
||||
meta.insert(
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(),
|
||||
serde_json::Value::String(MCP_TOOL_APPROVAL_PERSIST_SESSION.to_string()),
|
||||
);
|
||||
}
|
||||
(false, true) => {
|
||||
meta.insert(
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(),
|
||||
serde_json::Value::String(MCP_TOOL_APPROVAL_PERSIST_ALWAYS.to_string()),
|
||||
);
|
||||
}
|
||||
(false, false) => {}
|
||||
}
|
||||
if let Some(metadata) = metadata {
|
||||
if let Some(tool_title) = metadata.tool_title.as_ref() {
|
||||
@@ -864,15 +954,20 @@ fn parse_mcp_tool_approval_elicitation_response(
|
||||
};
|
||||
match response.action {
|
||||
ElicitationAction::Accept => {
|
||||
if response
|
||||
match response
|
||||
.meta
|
||||
.as_ref()
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.and_then(|meta| meta.get(MCP_TOOL_APPROVAL_PERSIST_KEY))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
== Some(MCP_TOOL_APPROVAL_PERSIST_SESSION)
|
||||
{
|
||||
return McpToolApprovalDecision::AcceptAndRemember;
|
||||
Some(MCP_TOOL_APPROVAL_PERSIST_SESSION) => {
|
||||
return McpToolApprovalDecision::AcceptForSession;
|
||||
}
|
||||
Some(MCP_TOOL_APPROVAL_PERSIST_ALWAYS) => {
|
||||
return McpToolApprovalDecision::AcceptAndRemember;
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
match parse_mcp_tool_approval_response(
|
||||
@@ -930,6 +1025,11 @@ fn parse_mcp_tool_approval_response(
|
||||
return McpToolApprovalDecision::Cancel;
|
||||
};
|
||||
if answers
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION)
|
||||
{
|
||||
McpToolApprovalDecision::AcceptForSession
|
||||
} else if answers
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER)
|
||||
{
|
||||
@@ -939,13 +1039,8 @@ fn parse_mcp_tool_approval_response(
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT)
|
||||
{
|
||||
McpToolApprovalDecision::Accept
|
||||
} else if answers
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_CANCEL)
|
||||
{
|
||||
McpToolApprovalDecision::Cancel
|
||||
} else {
|
||||
McpToolApprovalDecision::Decline
|
||||
McpToolApprovalDecision::Cancel
|
||||
}
|
||||
}
|
||||
|
||||
@@ -954,7 +1049,10 @@ fn normalize_approval_decision_for_mode(
|
||||
approval_mode: AppToolApproval,
|
||||
) -> McpToolApprovalDecision {
|
||||
if approval_mode == AppToolApproval::Prompt
|
||||
&& decision == McpToolApprovalDecision::AcceptAndRemember
|
||||
&& matches!(
|
||||
decision,
|
||||
McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember
|
||||
)
|
||||
{
|
||||
McpToolApprovalDecision::Accept
|
||||
} else {
|
||||
@@ -972,6 +1070,81 @@ async fn remember_mcp_tool_approval(sess: &Session, key: McpToolApprovalKey) {
|
||||
store.put(key, ReviewDecision::ApprovedForSession);
|
||||
}
|
||||
|
||||
async fn apply_mcp_tool_approval_decision(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
decision: McpToolApprovalDecision,
|
||||
session_approval_key: Option<McpToolApprovalKey>,
|
||||
persistent_approval_key: Option<McpToolApprovalKey>,
|
||||
) {
|
||||
match decision {
|
||||
McpToolApprovalDecision::AcceptForSession => {
|
||||
if let Some(key) = session_approval_key {
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
}
|
||||
}
|
||||
McpToolApprovalDecision::AcceptAndRemember => {
|
||||
if let Some(key) = persistent_approval_key {
|
||||
maybe_persist_mcp_tool_approval(sess, turn_context, key).await;
|
||||
} else if let Some(key) = session_approval_key {
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
}
|
||||
}
|
||||
McpToolApprovalDecision::Accept
|
||||
| McpToolApprovalDecision::Decline
|
||||
| McpToolApprovalDecision::Cancel => {}
|
||||
}
|
||||
}
|
||||
|
||||
async fn maybe_persist_mcp_tool_approval(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
key: McpToolApprovalKey,
|
||||
) {
|
||||
let Some(connector_id) = key.connector_id.clone() else {
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
return;
|
||||
};
|
||||
let tool_name = key.tool_name.clone();
|
||||
|
||||
if let Err(err) =
|
||||
persist_codex_app_tool_approval(&turn_context.config.codex_home, &connector_id, &tool_name)
|
||||
.await
|
||||
{
|
||||
error!(
|
||||
error = %err,
|
||||
connector_id,
|
||||
tool_name,
|
||||
"failed to persist codex app tool approval"
|
||||
);
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
return;
|
||||
}
|
||||
|
||||
sess.reload_user_config_layer().await;
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
}
|
||||
|
||||
async fn persist_codex_app_tool_approval(
|
||||
codex_home: &Path,
|
||||
connector_id: &str,
|
||||
tool_name: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
ConfigEditsBuilder::new(codex_home)
|
||||
.with_edits([ConfigEdit::SetPath {
|
||||
segments: vec![
|
||||
"apps".to_string(),
|
||||
connector_id.to_string(),
|
||||
"tools".to_string(),
|
||||
tool_name.to_string(),
|
||||
"approval_mode".to_string(),
|
||||
],
|
||||
value: value("approve"),
|
||||
}])
|
||||
.apply()
|
||||
.await
|
||||
}
|
||||
|
||||
fn requires_mcp_tool_approval(annotations: &ToolAnnotations) -> bool {
|
||||
if annotations.destructive_hint == Some(true) {
|
||||
return true;
|
||||
@@ -1006,7 +1179,17 @@ async fn notify_mcp_tool_call_skip(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::codex::make_session_and_context;
|
||||
use crate::config::ConfigToml;
|
||||
use crate::config::types::AppConfig;
|
||||
use crate::config::types::AppToolConfig;
|
||||
use crate::config::types::AppToolsConfig;
|
||||
use crate::config::types::AppsConfigToml;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashMap;
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn annotations(
|
||||
read_only: Option<bool>,
|
||||
@@ -1039,6 +1222,16 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn prompt_options(
|
||||
allow_session_remember: bool,
|
||||
allow_persistent_approval: bool,
|
||||
) -> McpToolApprovalPromptOptions {
|
||||
McpToolApprovalPromptOptions {
|
||||
allow_session_remember,
|
||||
allow_persistent_approval,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_required_when_read_only_false_and_destructive() {
|
||||
let annotations = annotations(Some(false), Some(true), None);
|
||||
@@ -1058,7 +1251,14 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prompt_mode_does_not_allow_session_remember() {
|
||||
fn prompt_mode_does_not_allow_persistent_remember() {
|
||||
assert_eq!(
|
||||
normalize_approval_decision_for_mode(
|
||||
McpToolApprovalDecision::AcceptForSession,
|
||||
AppToolApproval::Prompt,
|
||||
),
|
||||
McpToolApprovalDecision::Accept
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_approval_decision_for_mode(
|
||||
McpToolApprovalDecision::AcceptAndRemember,
|
||||
@@ -1077,7 +1277,7 @@ mod tests {
|
||||
Some("Run Action"),
|
||||
None,
|
||||
Some(&annotations(Some(false), Some(true), None)),
|
||||
true,
|
||||
prompt_options(false, false),
|
||||
);
|
||||
|
||||
assert_eq!(question.header, "Approve app tool call?");
|
||||
@@ -1086,7 +1286,7 @@ mod tests {
|
||||
"The custom_server MCP server wants to run the tool \"Run Action\", which may modify or delete data. Allow this action?"
|
||||
);
|
||||
assert!(
|
||||
question
|
||||
!question
|
||||
.options
|
||||
.expect("options")
|
||||
.into_iter()
|
||||
@@ -1104,7 +1304,7 @@ mod tests {
|
||||
Some("Run Action"),
|
||||
None,
|
||||
Some(&annotations(Some(false), Some(true), None)),
|
||||
true,
|
||||
prompt_options(true, true),
|
||||
);
|
||||
|
||||
assert!(
|
||||
@@ -1114,6 +1314,149 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trusted_codex_apps_tool_question_offers_always_allow() {
|
||||
let question = build_mcp_tool_approval_question(
|
||||
"q".to_string(),
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"run_action",
|
||||
Some("Run Action"),
|
||||
Some("Calendar"),
|
||||
Some(&annotations(Some(false), Some(true), None)),
|
||||
prompt_options(true, true),
|
||||
);
|
||||
let options = question.options.expect("options");
|
||||
|
||||
assert!(options.iter().any(|option| {
|
||||
option.label == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION
|
||||
&& option.description == "Run the tool and remember this choice for this session."
|
||||
}));
|
||||
assert!(options.iter().any(|option| {
|
||||
option.label == MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER
|
||||
&& option.description
|
||||
== "Run the tool and remember this choice for future tool calls."
|
||||
}));
|
||||
assert_eq!(
|
||||
options
|
||||
.into_iter()
|
||||
.map(|option| option.label)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
MCP_TOOL_APPROVAL_ACCEPT.to_string(),
|
||||
MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(),
|
||||
MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER.to_string(),
|
||||
MCP_TOOL_APPROVAL_CANCEL.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_tool_question_without_elicitation_omits_always_allow() {
|
||||
let session_key = McpToolApprovalKey {
|
||||
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
connector_id: Some("calendar".to_string()),
|
||||
tool_name: "run_action".to_string(),
|
||||
};
|
||||
let persistent_key = session_key.clone();
|
||||
let question = build_mcp_tool_approval_question(
|
||||
"q".to_string(),
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"run_action",
|
||||
Some("Run Action"),
|
||||
Some("Calendar"),
|
||||
Some(&annotations(Some(false), Some(true), None)),
|
||||
mcp_tool_approval_prompt_options(Some(&session_key), Some(&persistent_key), false),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
question
|
||||
.options
|
||||
.expect("options")
|
||||
.into_iter()
|
||||
.map(|option| option.label)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
MCP_TOOL_APPROVAL_ACCEPT.to_string(),
|
||||
MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(),
|
||||
MCP_TOOL_APPROVAL_CANCEL.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_mcp_tool_question_offers_session_remember_without_always_allow() {
|
||||
let question = build_mcp_tool_approval_question(
|
||||
"q".to_string(),
|
||||
"custom_server",
|
||||
"run_action",
|
||||
Some("Run Action"),
|
||||
None,
|
||||
Some(&annotations(Some(false), Some(true), None)),
|
||||
prompt_options(true, false),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
question
|
||||
.options
|
||||
.expect("options")
|
||||
.into_iter()
|
||||
.map(|option| option.label)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
MCP_TOOL_APPROVAL_ACCEPT.to_string(),
|
||||
MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(),
|
||||
MCP_TOOL_APPROVAL_CANCEL.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_servers_keep_session_remember_without_persistent_approval() {
|
||||
let invocation = McpInvocation {
|
||||
server: "custom_server".to_string(),
|
||||
tool: "run_action".to_string(),
|
||||
arguments: None,
|
||||
};
|
||||
let expected = McpToolApprovalKey {
|
||||
server: "custom_server".to_string(),
|
||||
connector_id: None,
|
||||
tool_name: "run_action".to_string(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
session_mcp_tool_approval_key(&invocation, None, AppToolApproval::Auto),
|
||||
Some(expected)
|
||||
);
|
||||
assert_eq!(
|
||||
persistent_mcp_tool_approval_key(&invocation, None, AppToolApproval::Auto),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_connectors_support_persistent_approval() {
|
||||
let invocation = McpInvocation {
|
||||
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
tool: "calendar/list_events".to_string(),
|
||||
arguments: None,
|
||||
};
|
||||
let metadata = approval_metadata(Some("calendar"), Some("Calendar"), None, None, None);
|
||||
let expected = McpToolApprovalKey {
|
||||
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
connector_id: Some("calendar".to_string()),
|
||||
tool_name: "calendar/list_events".to_string(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
session_mcp_tool_approval_key(&invocation, Some(&metadata), AppToolApproval::Auto),
|
||||
Some(expected.clone())
|
||||
);
|
||||
assert_eq!(
|
||||
persistent_mcp_tool_approval_key(&invocation, Some(&metadata), AppToolApproval::Auto),
|
||||
Some(expected)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_mcp_tool_result_for_model_rewrites_image_content() {
|
||||
let result = Ok(CallToolResult {
|
||||
@@ -1194,7 +1537,12 @@ mod tests {
|
||||
#[test]
|
||||
fn approval_elicitation_meta_marks_tool_approvals() {
|
||||
assert_eq!(
|
||||
build_mcp_tool_approval_elicitation_meta("custom_server", None, None, false),
|
||||
build_mcp_tool_approval_elicitation_meta(
|
||||
"custom_server",
|
||||
None,
|
||||
None,
|
||||
prompt_options(false, false),
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL,
|
||||
}))
|
||||
@@ -1202,7 +1550,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_elicitation_meta_keeps_session_persist_behavior() {
|
||||
fn approval_elicitation_meta_keeps_session_persist_behavior_for_custom_servers() {
|
||||
assert_eq!(
|
||||
build_mcp_tool_approval_elicitation_meta(
|
||||
"custom_server",
|
||||
@@ -1214,7 +1562,7 @@ mod tests {
|
||||
Some("Runs the selected action."),
|
||||
)),
|
||||
Some(&serde_json::json!({"id": 1})),
|
||||
true,
|
||||
prompt_options(true, false),
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL,
|
||||
@@ -1335,7 +1683,7 @@ mod tests {
|
||||
Some(&serde_json::json!({
|
||||
"calendar_id": "primary",
|
||||
})),
|
||||
false,
|
||||
prompt_options(false, false),
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL,
|
||||
@@ -1353,7 +1701,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_elicitation_meta_merges_session_persist_with_connector_source() {
|
||||
fn approval_elicitation_meta_merges_session_and_always_persist_with_connector_source() {
|
||||
assert_eq!(
|
||||
build_mcp_tool_approval_elicitation_meta(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
@@ -1367,11 +1715,14 @@ mod tests {
|
||||
Some(&serde_json::json!({
|
||||
"calendar_id": "primary",
|
||||
})),
|
||||
true,
|
||||
prompt_options(true, true),
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL,
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY: MCP_TOOL_APPROVAL_PERSIST_SESSION,
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY: [
|
||||
MCP_TOOL_APPROVAL_PERSIST_SESSION,
|
||||
MCP_TOOL_APPROVAL_PERSIST_ALWAYS,
|
||||
],
|
||||
MCP_TOOL_APPROVAL_SOURCE_KEY: MCP_TOOL_APPROVAL_SOURCE_CONNECTOR,
|
||||
MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: "calendar",
|
||||
MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY: "Calendar",
|
||||
@@ -1401,6 +1752,22 @@ mod tests {
|
||||
assert_eq!(response, McpToolApprovalDecision::Decline);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepted_elicitation_response_uses_always_persist_meta() {
|
||||
let response = parse_mcp_tool_approval_elicitation_response(
|
||||
Some(ElicitationResponse {
|
||||
action: ElicitationAction::Accept,
|
||||
content: None,
|
||||
meta: Some(serde_json::json!({
|
||||
MCP_TOOL_APPROVAL_PERSIST_KEY: MCP_TOOL_APPROVAL_PERSIST_ALWAYS,
|
||||
})),
|
||||
}),
|
||||
"approval",
|
||||
);
|
||||
|
||||
assert_eq!(response, McpToolApprovalDecision::AcceptAndRemember);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepted_elicitation_response_uses_session_persist_meta() {
|
||||
let response = parse_mcp_tool_approval_elicitation_response(
|
||||
@@ -1414,7 +1781,7 @@ mod tests {
|
||||
"approval",
|
||||
);
|
||||
|
||||
assert_eq!(response, McpToolApprovalDecision::AcceptAndRemember);
|
||||
assert_eq!(response, McpToolApprovalDecision::AcceptForSession);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1430,4 +1797,83 @@ mod tests {
|
||||
|
||||
assert_eq!(response, McpToolApprovalDecision::Accept);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persist_codex_app_tool_approval_writes_tool_override() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
persist_codex_app_tool_approval(tmp.path(), "calendar", "calendar/list_events")
|
||||
.await
|
||||
.expect("persist approval");
|
||||
|
||||
let contents =
|
||||
std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config");
|
||||
let parsed: ConfigToml = toml::from_str(&contents).expect("parse config");
|
||||
|
||||
assert_eq!(
|
||||
parsed.apps,
|
||||
Some(AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
destructive_enabled: None,
|
||||
open_world_enabled: None,
|
||||
default_tools_approval_mode: None,
|
||||
default_tools_enabled: None,
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"calendar/list_events".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: None,
|
||||
approval_mode: Some(AppToolApproval::Approve),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
})
|
||||
);
|
||||
assert!(contents.contains("[apps.calendar.tools.\"calendar/list_events\"]"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn maybe_persist_mcp_tool_approval_reloads_session_config() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let codex_home = session.codex_home().await;
|
||||
std::fs::create_dir_all(&codex_home).expect("create codex home");
|
||||
let key = McpToolApprovalKey {
|
||||
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
connector_id: Some("calendar".to_string()),
|
||||
tool_name: "calendar/list_events".to_string(),
|
||||
};
|
||||
|
||||
maybe_persist_mcp_tool_approval(&session, &turn_context, key.clone()).await;
|
||||
|
||||
let config = session.get_config().await;
|
||||
let apps_toml = config
|
||||
.config_layer_stack
|
||||
.effective_config()
|
||||
.as_table()
|
||||
.and_then(|table| table.get("apps"))
|
||||
.cloned()
|
||||
.expect("apps table");
|
||||
let apps = AppsConfigToml::deserialize(apps_toml).expect("deserialize apps config");
|
||||
let tool = apps
|
||||
.apps
|
||||
.get("calendar")
|
||||
.and_then(|app| app.tools.as_ref())
|
||||
.and_then(|tools| tools.tools.get("calendar/list_events"))
|
||||
.expect("calendar/list_events tool config exists");
|
||||
|
||||
assert_eq!(
|
||||
tool,
|
||||
&AppToolConfig {
|
||||
enabled: None,
|
||||
approval_mode: Some(AppToolApproval::Approve),
|
||||
}
|
||||
);
|
||||
assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true);
|
||||
}
|
||||
}
|
||||
|
||||
+139
-13
@@ -768,12 +768,47 @@ impl App {
|
||||
}
|
||||
|
||||
async fn refresh_in_memory_config_from_disk(&mut self) -> Result<()> {
|
||||
let mut config = self.rebuild_config_for_cwd(self.config.cwd.clone()).await?;
|
||||
let mut config = self
|
||||
.rebuild_config_for_cwd(self.chat_widget.config_ref().cwd.clone())
|
||||
.await?;
|
||||
self.apply_runtime_policy_overrides(&mut config);
|
||||
self.config = config;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn refresh_in_memory_config_from_disk_best_effort(&mut self, action: &str) {
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
action,
|
||||
"failed to refresh config before thread transition; continuing with current in-memory config"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async fn rebuild_config_for_resume_or_fallback(
|
||||
&mut self,
|
||||
current_cwd: &Path,
|
||||
resume_cwd: PathBuf,
|
||||
) -> Result<Config> {
|
||||
match self.rebuild_config_for_cwd(resume_cwd.clone()).await {
|
||||
Ok(config) => Ok(config),
|
||||
Err(err) => {
|
||||
if crate::cwds_differ(current_cwd, &resume_cwd) {
|
||||
Err(err)
|
||||
} else {
|
||||
let resume_cwd_display = resume_cwd.display().to_string();
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
cwd = %resume_cwd_display,
|
||||
"failed to rebuild config for same-cwd resume; using current in-memory config"
|
||||
);
|
||||
Ok(self.config.clone())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_runtime_policy_overrides(&mut self, config: &mut Config) {
|
||||
if let Some(policy) = self.runtime_approval_policy_override.as_ref()
|
||||
&& let Err(err) = config.permissions.approval_policy.set(*policy)
|
||||
@@ -1482,6 +1517,8 @@ impl App {
|
||||
async fn start_fresh_session_with_summary_hint(&mut self, tui: &mut tui::Tui) {
|
||||
// Start a fresh in-memory session while preserving resumability via persisted rollout
|
||||
// history.
|
||||
self.refresh_in_memory_config_from_disk_best_effort("starting a new thread")
|
||||
.await;
|
||||
let model = self.chat_widget.current_model().to_string();
|
||||
let config = self.fresh_session_config();
|
||||
let summary = session_summary(
|
||||
@@ -2078,19 +2115,17 @@ impl App {
|
||||
return Ok(AppRunControl::Exit(ExitReason::UserRequested));
|
||||
}
|
||||
};
|
||||
let mut resume_config = if crate::cwds_differ(¤t_cwd, &resume_cwd) {
|
||||
match self.rebuild_config_for_cwd(resume_cwd).await {
|
||||
Ok(cfg) => cfg,
|
||||
Err(err) => {
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to rebuild configuration for resume: {err}"
|
||||
));
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
let mut resume_config = match self
|
||||
.rebuild_config_for_resume_or_fallback(¤t_cwd, resume_cwd)
|
||||
.await
|
||||
{
|
||||
Ok(cfg) => cfg,
|
||||
Err(err) => {
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to rebuild configuration for resume: {err}"
|
||||
));
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
} else {
|
||||
// No rebuild needed: current_cwd comes from self.config.cwd.
|
||||
self.config.clone()
|
||||
};
|
||||
self.apply_runtime_policy_overrides(&mut resume_config);
|
||||
let summary = session_summary(
|
||||
@@ -2165,6 +2200,8 @@ impl App {
|
||||
self.chat_widget
|
||||
.add_plain_history_lines(vec!["/fork".magenta().into()]);
|
||||
if let Some(path) = self.chat_widget.rollout_path() {
|
||||
self.refresh_in_memory_config_from_disk_best_effort("forking the thread")
|
||||
.await;
|
||||
// Fresh threads expose a precomputed path, but the file is
|
||||
// materialized lazily on first user message.
|
||||
if path.exists() {
|
||||
@@ -5907,6 +5944,95 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_in_memory_config_from_disk_best_effort_keeps_current_config_on_error()
|
||||
-> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf();
|
||||
std::fs::write(codex_home.path().join("config.toml"), "[broken")?;
|
||||
let original_config = app.config.clone();
|
||||
|
||||
app.refresh_in_memory_config_from_disk_best_effort("starting a new thread")
|
||||
.await;
|
||||
|
||||
assert_eq!(app.config, original_config);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_in_memory_config_from_disk_uses_active_chat_widget_cwd() -> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let original_cwd = app.config.cwd.clone();
|
||||
let next_cwd_tmp = tempdir()?;
|
||||
let next_cwd = next_cwd_tmp.path().to_path_buf();
|
||||
|
||||
app.chat_widget.handle_codex_event(Event {
|
||||
id: String::new(),
|
||||
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
|
||||
session_id: ThreadId::new(),
|
||||
forked_from_id: None,
|
||||
thread_name: None,
|
||||
model: "gpt-test".to_string(),
|
||||
model_provider_id: "test-provider".to_string(),
|
||||
service_tier: None,
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
cwd: next_cwd.clone(),
|
||||
reasoning_effort: None,
|
||||
history_log_id: 0,
|
||||
history_entry_count: 0,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: Some(PathBuf::new()),
|
||||
}),
|
||||
});
|
||||
|
||||
assert_eq!(app.chat_widget.config_ref().cwd, next_cwd);
|
||||
assert_eq!(app.config.cwd, original_cwd);
|
||||
|
||||
app.refresh_in_memory_config_from_disk().await?;
|
||||
|
||||
assert_eq!(app.config.cwd, app.chat_widget.config_ref().cwd);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rebuild_config_for_resume_or_fallback_uses_current_config_on_same_cwd_error()
|
||||
-> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf();
|
||||
std::fs::write(codex_home.path().join("config.toml"), "[broken")?;
|
||||
let current_config = app.config.clone();
|
||||
let current_cwd = current_config.cwd.clone();
|
||||
|
||||
let resume_config = app
|
||||
.rebuild_config_for_resume_or_fallback(¤t_cwd, current_cwd.clone())
|
||||
.await?;
|
||||
|
||||
assert_eq!(resume_config, current_config);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rebuild_config_for_resume_or_fallback_errors_when_cwd_changes() -> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf();
|
||||
std::fs::write(codex_home.path().join("config.toml"), "[broken")?;
|
||||
let current_cwd = app.config.cwd.clone();
|
||||
let next_cwd_tmp = tempdir()?;
|
||||
let next_cwd = next_cwd_tmp.path().to_path_buf();
|
||||
|
||||
let result = app
|
||||
.rebuild_config_for_resume_or_fallback(¤t_cwd, next_cwd)
|
||||
.await;
|
||||
|
||||
assert!(result.is_err());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sync_tui_theme_selection_updates_chat_widget_config_copy() {
|
||||
let mut app = make_test_app().await;
|
||||
|
||||
@@ -49,12 +49,14 @@ const MIN_OVERLAY_HEIGHT: u16 = 8;
|
||||
const APPROVAL_FIELD_ID: &str = "__approval";
|
||||
const APPROVAL_ACCEPT_ONCE_VALUE: &str = "accept";
|
||||
const APPROVAL_ACCEPT_SESSION_VALUE: &str = "accept_session";
|
||||
const APPROVAL_ACCEPT_ALWAYS_VALUE: &str = "accept_always";
|
||||
const APPROVAL_DECLINE_VALUE: &str = "decline";
|
||||
const APPROVAL_CANCEL_VALUE: &str = "cancel";
|
||||
const APPROVAL_META_KIND_KEY: &str = "codex_approval_kind";
|
||||
const APPROVAL_META_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call";
|
||||
const APPROVAL_PERSIST_KEY: &str = "persist";
|
||||
const APPROVAL_PERSIST_SESSION_VALUE: &str = "session";
|
||||
const APPROVAL_PERSIST_ALWAYS_VALUE: &str = "always";
|
||||
|
||||
#[derive(Clone, PartialEq, Default)]
|
||||
struct ComposerDraft {
|
||||
@@ -181,29 +183,49 @@ impl McpServerElicitationFormRequest {
|
||||
.and_then(Value::as_object)
|
||||
.is_some_and(serde_json::Map::is_empty)
|
||||
});
|
||||
let is_tool_approval_action =
|
||||
is_tool_approval && (requested_schema.is_null() || is_empty_object_schema);
|
||||
|
||||
let (response_mode, fields) =
|
||||
if requested_schema.is_null() || (is_tool_approval && is_empty_object_schema) {
|
||||
let mut options = vec![McpServerElicitationOption {
|
||||
label: "Approve Once".to_string(),
|
||||
description: Some("Run the tool and continue.".to_string()),
|
||||
value: Value::String(APPROVAL_ACCEPT_ONCE_VALUE.to_string()),
|
||||
}];
|
||||
if meta
|
||||
.as_ref()
|
||||
.and_then(Value::as_object)
|
||||
.and_then(|meta| meta.get(APPROVAL_PERSIST_KEY))
|
||||
.and_then(Value::as_str)
|
||||
== Some(APPROVAL_PERSIST_SESSION_VALUE)
|
||||
{
|
||||
options.push(McpServerElicitationOption {
|
||||
label: "Approve this Session".to_string(),
|
||||
description: Some(
|
||||
"Run the tool and remember this choice for this session.".to_string(),
|
||||
),
|
||||
value: Value::String(APPROVAL_ACCEPT_SESSION_VALUE.to_string()),
|
||||
});
|
||||
}
|
||||
let (response_mode, fields) = if requested_schema.is_null()
|
||||
|| (is_tool_approval && is_empty_object_schema)
|
||||
{
|
||||
let mut options = vec![McpServerElicitationOption {
|
||||
label: "Approve Once".to_string(),
|
||||
description: Some("Run the tool and continue.".to_string()),
|
||||
value: Value::String(APPROVAL_ACCEPT_ONCE_VALUE.to_string()),
|
||||
}];
|
||||
if is_tool_approval_action
|
||||
&& tool_approval_supports_persist_mode(
|
||||
meta.as_ref(),
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
)
|
||||
{
|
||||
options.push(McpServerElicitationOption {
|
||||
label: "Approve this session".to_string(),
|
||||
description: Some(
|
||||
"Run the tool and remember this choice for this session.".to_string(),
|
||||
),
|
||||
value: Value::String(APPROVAL_ACCEPT_SESSION_VALUE.to_string()),
|
||||
});
|
||||
}
|
||||
if is_tool_approval_action
|
||||
&& tool_approval_supports_persist_mode(meta.as_ref(), APPROVAL_PERSIST_ALWAYS_VALUE)
|
||||
{
|
||||
options.push(McpServerElicitationOption {
|
||||
label: "Always allow".to_string(),
|
||||
description: Some(
|
||||
"Run the tool and remember this choice for future tool calls.".to_string(),
|
||||
),
|
||||
value: Value::String(APPROVAL_ACCEPT_ALWAYS_VALUE.to_string()),
|
||||
});
|
||||
}
|
||||
if is_tool_approval_action {
|
||||
options.push(McpServerElicitationOption {
|
||||
label: "Cancel".to_string(),
|
||||
description: Some("Cancel this tool call".to_string()),
|
||||
value: Value::String(APPROVAL_CANCEL_VALUE.to_string()),
|
||||
});
|
||||
} else {
|
||||
options.extend([
|
||||
McpServerElicitationOption {
|
||||
label: "Deny".to_string(),
|
||||
@@ -216,25 +238,26 @@ impl McpServerElicitationFormRequest {
|
||||
value: Value::String(APPROVAL_CANCEL_VALUE.to_string()),
|
||||
},
|
||||
]);
|
||||
(
|
||||
McpServerElicitationResponseMode::ApprovalAction,
|
||||
vec![McpServerElicitationField {
|
||||
id: APPROVAL_FIELD_ID.to_string(),
|
||||
label: String::new(),
|
||||
prompt: String::new(),
|
||||
required: true,
|
||||
input: McpServerElicitationFieldInput::Select {
|
||||
options,
|
||||
default_idx: Some(0),
|
||||
},
|
||||
}],
|
||||
)
|
||||
} else {
|
||||
(
|
||||
McpServerElicitationResponseMode::FormContent,
|
||||
parse_fields_from_schema(&requested_schema)?,
|
||||
)
|
||||
};
|
||||
}
|
||||
(
|
||||
McpServerElicitationResponseMode::ApprovalAction,
|
||||
vec![McpServerElicitationField {
|
||||
id: APPROVAL_FIELD_ID.to_string(),
|
||||
label: String::new(),
|
||||
prompt: String::new(),
|
||||
required: true,
|
||||
input: McpServerElicitationFieldInput::Select {
|
||||
options,
|
||||
default_idx: Some(0),
|
||||
},
|
||||
}],
|
||||
)
|
||||
} else {
|
||||
(
|
||||
McpServerElicitationResponseMode::FormContent,
|
||||
parse_fields_from_schema(&requested_schema)?,
|
||||
)
|
||||
};
|
||||
|
||||
Some(Self {
|
||||
thread_id,
|
||||
@@ -247,6 +270,24 @@ impl McpServerElicitationFormRequest {
|
||||
}
|
||||
}
|
||||
|
||||
fn tool_approval_supports_persist_mode(meta: Option<&Value>, expected_mode: &str) -> bool {
|
||||
let Some(persist) = meta
|
||||
.and_then(Value::as_object)
|
||||
.and_then(|meta| meta.get(APPROVAL_PERSIST_KEY))
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
|
||||
match persist {
|
||||
Value::String(value) => value == expected_mode,
|
||||
Value::Array(values) => values
|
||||
.iter()
|
||||
.filter_map(Value::as_str)
|
||||
.any(|value| value == expected_mode),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_fields_from_schema(requested_schema: &Value) -> Option<Vec<McpServerElicitationField>> {
|
||||
let schema = requested_schema.as_object()?;
|
||||
if schema.get("type").and_then(Value::as_str) != Some("object") {
|
||||
@@ -856,6 +897,12 @@ impl McpServerElicitationOverlay {
|
||||
APPROVAL_PERSIST_KEY: APPROVAL_PERSIST_SESSION_VALUE,
|
||||
})),
|
||||
),
|
||||
Some(APPROVAL_ACCEPT_ALWAYS_VALUE) => (
|
||||
ElicitationAction::Accept,
|
||||
Some(serde_json::json!({
|
||||
APPROVAL_PERSIST_KEY: APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
})),
|
||||
),
|
||||
Some(APPROVAL_DECLINE_VALUE) => (ElicitationAction::Decline, None),
|
||||
Some(APPROVAL_CANCEL_VALUE) => (ElicitationAction::Cancel, None),
|
||||
_ => (ElicitationAction::Cancel, None),
|
||||
@@ -1414,15 +1461,20 @@ mod tests {
|
||||
})
|
||||
}
|
||||
|
||||
fn tool_approval_meta(include_session_persist: bool) -> Option<Value> {
|
||||
fn tool_approval_meta(persist_modes: &[&str]) -> Option<Value> {
|
||||
let mut meta = serde_json::Map::from_iter([(
|
||||
APPROVAL_META_KIND_KEY.to_string(),
|
||||
Value::String(APPROVAL_META_KIND_MCP_TOOL_CALL.to_string()),
|
||||
)]);
|
||||
if include_session_persist {
|
||||
if !persist_modes.is_empty() {
|
||||
meta.insert(
|
||||
APPROVAL_PERSIST_KEY.to_string(),
|
||||
Value::String(APPROVAL_PERSIST_SESSION_VALUE.to_string()),
|
||||
Value::Array(
|
||||
persist_modes
|
||||
.iter()
|
||||
.map(|mode| Value::String((*mode).to_string()))
|
||||
.collect(),
|
||||
),
|
||||
);
|
||||
}
|
||||
Some(Value::Object(meta))
|
||||
@@ -1581,7 +1633,7 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(false),
|
||||
tool_approval_meta(&[]),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -1606,13 +1658,6 @@ mod tests {
|
||||
description: Some("Run the tool and continue.".to_string()),
|
||||
value: Value::String(APPROVAL_ACCEPT_ONCE_VALUE.to_string()),
|
||||
},
|
||||
McpServerElicitationOption {
|
||||
label: "Deny".to_string(),
|
||||
description: Some(
|
||||
"Decline this tool call and continue.".to_string(),
|
||||
),
|
||||
value: Value::String(APPROVAL_DECLINE_VALUE.to_string()),
|
||||
},
|
||||
McpServerElicitationOption {
|
||||
label: "Cancel".to_string(),
|
||||
description: Some("Cancel this tool call".to_string()),
|
||||
@@ -1696,7 +1741,10 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(true),
|
||||
tool_approval_meta(&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
]),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -1731,6 +1779,53 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_tool_approval_schema_always_allow_sets_persist_meta() {
|
||||
let (tx, mut rx) = test_sender();
|
||||
let thread_id = ThreadId::default();
|
||||
let request = McpServerElicitationFormRequest::from_event(
|
||||
thread_id,
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
]),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
let mut overlay = McpServerElicitationOverlay::new(request, tx, true, false, false);
|
||||
|
||||
if let Some(answer) = overlay.current_answer_mut() {
|
||||
answer.selection.selected_idx = Some(2);
|
||||
}
|
||||
overlay.select_current_option(true);
|
||||
overlay.submit_answers();
|
||||
|
||||
let event = rx.try_recv().expect("expected resolution");
|
||||
let AppEvent::SubmitThreadOp {
|
||||
thread_id: resolved_thread_id,
|
||||
op,
|
||||
} = event
|
||||
else {
|
||||
panic!("expected SubmitThreadOp");
|
||||
};
|
||||
assert_eq!(resolved_thread_id, thread_id);
|
||||
assert_eq!(
|
||||
op,
|
||||
Op::ResolveElicitation {
|
||||
server_name: "server-1".to_string(),
|
||||
request_id: McpRequestId::String("request-1".to_string()),
|
||||
decision: ElicitationAction::Accept,
|
||||
content: None,
|
||||
meta: Some(serde_json::json!({
|
||||
APPROVAL_PERSIST_KEY: APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
})),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ctrl_c_cancels_elicitation() {
|
||||
let (tx, mut rx) = test_sender();
|
||||
@@ -1886,7 +1981,7 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(false),
|
||||
tool_approval_meta(&[]),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -1899,14 +1994,17 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_form_tool_approval_with_session_persist_snapshot() {
|
||||
fn approval_form_tool_approval_with_persist_options_snapshot() {
|
||||
let (tx, _rx) = test_sender();
|
||||
let request = McpServerElicitationFormRequest::from_event(
|
||||
ThreadId::default(),
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(true),
|
||||
tool_approval_meta(&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
]),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
|
||||
+2
-2
@@ -6,8 +6,8 @@ expression: "render_snapshot(&overlay, Rect::new(0, 0, 120, 16))"
|
||||
Field 1/1
|
||||
Allow this request?
|
||||
› 1. Approve Once Run the tool and continue.
|
||||
2. Approve this Session Run the tool and remember this choice for this session.
|
||||
3. Deny Decline this tool call and continue.
|
||||
2. Approve this session Run the tool and remember this choice for this session.
|
||||
3. Always allow Run the tool and remember this choice for future tool calls.
|
||||
4. Cancel Cancel this tool call
|
||||
|
||||
|
||||
|
||||
+2
-2
@@ -6,8 +6,8 @@ expression: "render_snapshot(&overlay, Rect::new(0, 0, 120, 16))"
|
||||
Field 1/1
|
||||
Allow this request?
|
||||
› 1. Approve Once Run the tool and continue.
|
||||
2. Deny Decline this tool call and continue.
|
||||
3. Cancel Cancel this tool call
|
||||
2. Cancel Cancel this tool call
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user