mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
[codex-rs] Add writes app approval mode (#30482)
## Summary - Adds `writes` to `AppToolApproval` and exposes it through config and app-server schemas, including `[apps._default].default_tools_approval_mode`. - In `writes`, tools with `readOnlyHint = true` skip approval; all other tools prompt, including non-destructive writes and tools without annotations. - Prevents session or persistent approval choices in this mode so later writes still prompt. ## Why `auto` only prompts for risk-hinted actions, while `prompt` also interrupts reads. Apps need a middle mode that gates writes without prompting for declared read-only actions. ## Validation - `just write-config-schema` - `just write-app-server-schema` - `just fmt` - `just test -p codex-core mcp_turn_metadata` (4 passed) - `just test -p codex-core writes_mode` (2 passed) - `just test -p codex-app-server config_read_includes_apps` (1 passed) - `just test -p codex-app-server-protocol` (251 passed) - `just test -p codex-config` (200 passed) - `just test -p codex-cli` (300 passed) - `just fix -p codex-core -p codex-config -p codex-app-server-protocol -p codex-app-server -p codex-cli`
This commit is contained in:
+1
@@ -6728,6 +6728,7 @@
|
||||
"enum": [
|
||||
"auto",
|
||||
"prompt",
|
||||
"writes",
|
||||
"approve"
|
||||
],
|
||||
"type": "string"
|
||||
|
||||
+1
@@ -854,6 +854,7 @@
|
||||
"enum": [
|
||||
"auto",
|
||||
"prompt",
|
||||
"writes",
|
||||
"approve"
|
||||
],
|
||||
"type": "string"
|
||||
|
||||
@@ -78,6 +78,7 @@
|
||||
"enum": [
|
||||
"auto",
|
||||
"prompt",
|
||||
"writes",
|
||||
"approve"
|
||||
],
|
||||
"type": "string"
|
||||
|
||||
@@ -2,4 +2,4 @@
|
||||
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
|
||||
export type AppToolApproval = "auto" | "prompt" | "approve";
|
||||
export type AppToolApproval = "auto" | "prompt" | "writes" | "approve";
|
||||
|
||||
@@ -163,6 +163,7 @@ pub struct AnalyticsConfig {
|
||||
pub enum AppToolApproval {
|
||||
Auto,
|
||||
Prompt,
|
||||
Writes,
|
||||
Approve,
|
||||
}
|
||||
|
||||
|
||||
@@ -1879,7 +1879,9 @@ review when allowed by configuration requirements.
|
||||
|
||||
Use `apps._default.default_tools_approval_mode` to set the approval mode for
|
||||
tools without a per-app or per-tool override. Supported values are `"auto"`,
|
||||
`"prompt"`, and `"approve"`. Tool-level `approval_mode` takes precedence over
|
||||
`"prompt"`, `"writes"`, and `"approve"`. The `"writes"` mode prompts for tools
|
||||
that do not advertise `readOnlyHint = true` and skips declared read-only tools.
|
||||
Tool-level `approval_mode` takes precedence over
|
||||
the per-app `default_tools_approval_mode`, which takes precedence over the
|
||||
`apps._default` value. Managed tool requirements take precedence over all of
|
||||
these settings. When none are configured, the mode defaults to `"auto"`.
|
||||
|
||||
@@ -400,7 +400,7 @@ async fn config_read_includes_apps() -> Result<()> {
|
||||
r#"
|
||||
[apps._default]
|
||||
approvals_reviewer = "auto_review"
|
||||
default_tools_approval_mode = "approve"
|
||||
default_tools_approval_mode = "writes"
|
||||
|
||||
[apps.app1]
|
||||
enabled = false
|
||||
@@ -440,7 +440,7 @@ default_tools_approval_mode = "prompt"
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: true,
|
||||
default_tools_approval_mode: Some(AppToolApproval::Approve),
|
||||
default_tools_approval_mode: Some(AppToolApproval::Writes),
|
||||
}),
|
||||
apps: std::collections::HashMap::from([(
|
||||
"app1".to_string(),
|
||||
|
||||
@@ -963,6 +963,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
|
||||
let approval_mode = match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
};
|
||||
println!(" default_tools_approval_mode: {approval_mode}");
|
||||
|
||||
@@ -198,6 +198,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
|
||||
entry["default_tools_approval_mode"] = value(match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
});
|
||||
}
|
||||
@@ -242,6 +243,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
|
||||
tool_entry["approval_mode"] = value(match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ pub enum AppToolApproval {
|
||||
#[default]
|
||||
Auto,
|
||||
Prompt,
|
||||
Writes,
|
||||
Approve,
|
||||
}
|
||||
|
||||
|
||||
@@ -150,6 +150,7 @@
|
||||
"enum": [
|
||||
"auto",
|
||||
"prompt",
|
||||
"writes",
|
||||
"approve"
|
||||
],
|
||||
"type": "string"
|
||||
|
||||
@@ -121,6 +121,7 @@ fn serialize_mcp_server_table(config: &McpServerConfig) -> TomlTable {
|
||||
entry["default_tools_approval_mode"] = value(match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
});
|
||||
}
|
||||
@@ -173,6 +174,7 @@ fn serialize_mcp_server_tool(config: &McpServerToolConfig) -> TomlItem {
|
||||
entry["approval_mode"] = value(match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1223,8 +1223,7 @@ async fn maybe_request_mcp_tool_approval(
|
||||
}
|
||||
|
||||
let annotations = metadata.and_then(|metadata| metadata.annotations.as_ref());
|
||||
let approval_required = requires_mcp_tool_approval(annotations);
|
||||
if !approval_required && approval_mode != AppToolApproval::Prompt {
|
||||
if !requires_mcp_tool_approval_for_mode(annotations, approval_mode) {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -1927,12 +1926,13 @@ fn normalize_approval_decision_for_mode(
|
||||
decision: McpToolApprovalDecision,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> McpToolApprovalDecision {
|
||||
if approval_mode == AppToolApproval::Prompt
|
||||
&& matches!(
|
||||
decision,
|
||||
McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember
|
||||
)
|
||||
{
|
||||
if matches!(
|
||||
approval_mode,
|
||||
AppToolApproval::Prompt | AppToolApproval::Writes
|
||||
) && matches!(
|
||||
decision,
|
||||
McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember
|
||||
) {
|
||||
McpToolApprovalDecision::Accept
|
||||
} else {
|
||||
decision
|
||||
@@ -2179,6 +2179,20 @@ fn requires_mcp_tool_approval(annotations: Option<&ToolAnnotations>) -> bool {
|
||||
.unwrap_or(true)
|
||||
}
|
||||
|
||||
fn requires_mcp_tool_approval_for_mode(
|
||||
annotations: Option<&ToolAnnotations>,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> bool {
|
||||
match approval_mode {
|
||||
AppToolApproval::Auto => requires_mcp_tool_approval(annotations),
|
||||
AppToolApproval::Prompt => true,
|
||||
AppToolApproval::Writes => !annotations
|
||||
.and_then(|annotations| annotations.read_only_hint)
|
||||
.unwrap_or(false),
|
||||
AppToolApproval::Approve => false,
|
||||
}
|
||||
}
|
||||
|
||||
async fn notify_mcp_tool_call_skip(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
|
||||
@@ -398,23 +398,47 @@ fn approval_not_required_when_read_only_and_other_hints_are_absent() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prompt_mode_does_not_allow_persistent_remember() {
|
||||
fn writes_mode_requires_approval_for_non_read_only_tools() {
|
||||
let annotations = annotations(Some(false), Some(false), Some(false));
|
||||
assert_eq!(
|
||||
normalize_approval_decision_for_mode(
|
||||
McpToolApprovalDecision::AcceptForSession,
|
||||
AppToolApproval::Prompt,
|
||||
),
|
||||
McpToolApprovalDecision::Accept
|
||||
requires_mcp_tool_approval_for_mode(Some(&annotations), AppToolApproval::Writes),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_approval_decision_for_mode(
|
||||
McpToolApprovalDecision::AcceptAndRemember,
|
||||
AppToolApproval::Prompt,
|
||||
),
|
||||
McpToolApprovalDecision::Accept
|
||||
requires_mcp_tool_approval_for_mode(/*annotations*/ None, AppToolApproval::Writes),
|
||||
true
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn writes_mode_does_not_require_approval_for_read_only_tools() {
|
||||
let annotations = annotations(Some(true), Some(true), Some(true));
|
||||
assert_eq!(
|
||||
requires_mcp_tool_approval_for_mode(Some(&annotations), AppToolApproval::Writes),
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prompting_modes_do_not_allow_persistent_remember() {
|
||||
for approval_mode in [AppToolApproval::Prompt, AppToolApproval::Writes] {
|
||||
assert_eq!(
|
||||
normalize_approval_decision_for_mode(
|
||||
McpToolApprovalDecision::AcceptForSession,
|
||||
approval_mode,
|
||||
),
|
||||
McpToolApprovalDecision::Accept
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_approval_decision_for_mode(
|
||||
McpToolApprovalDecision::AcceptAndRemember,
|
||||
approval_mode,
|
||||
),
|
||||
McpToolApprovalDecision::Accept
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_tool_call_span_records_expected_fields() {
|
||||
let buffer: &'static std::sync::Mutex<Vec<u8>> =
|
||||
|
||||
@@ -20,6 +20,7 @@ use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::apps_test_server::AppsTestServer;
|
||||
use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL;
|
||||
use core_test_support::apps_test_server::SEARCH_CALENDAR_LIST_TOOL;
|
||||
use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE;
|
||||
use core_test_support::apps_test_server::recorded_apps_tool_call_by_call_id;
|
||||
use core_test_support::apps_test_server::search_capable_apps_builder;
|
||||
@@ -45,6 +46,7 @@ fn set_calendar_approval_mode(config: &mut Config, approval_mode: AppToolApprova
|
||||
let approval_mode = match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
};
|
||||
let user_config_path = config.codex_home.join("config.toml").abs();
|
||||
@@ -68,6 +70,7 @@ fn set_default_app_approval_mode_and_reviewer(
|
||||
let approval_mode = match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Writes => "writes",
|
||||
AppToolApproval::Approve => "approve",
|
||||
};
|
||||
let user_config_path = config.codex_home.join("config.toml").abs();
|
||||
@@ -333,6 +336,127 @@ async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guar
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let apps_server = AppsTestServer::mount(&server).await?;
|
||||
let read_call_id = "calendar-read";
|
||||
let write_call_id = "calendar-write";
|
||||
let list_args = serde_json::to_string(&json!({}))?;
|
||||
let create_args = serde_json::to_string(&json!({
|
||||
"title": "Lunch",
|
||||
"starts_at": "2026-03-10T12:00:00Z"
|
||||
}))?;
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-read"),
|
||||
ev_function_call_with_namespace(
|
||||
read_call_id,
|
||||
SEARCH_CALENDAR_NAMESPACE,
|
||||
SEARCH_CALENDAR_LIST_TOOL,
|
||||
&list_args,
|
||||
),
|
||||
ev_completed("resp-read"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-write"),
|
||||
ev_function_call_with_namespace(
|
||||
write_call_id,
|
||||
SEARCH_CALENDAR_NAMESPACE,
|
||||
SEARCH_CALENDAR_CREATE_TOOL,
|
||||
&create_args,
|
||||
),
|
||||
ev_completed("resp-write"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-done"),
|
||||
ev_assistant_message("msg-done", "done"),
|
||||
ev_completed("resp-done"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone())
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ToolCallMcpElicitation)
|
||||
.expect("test config should allow feature update");
|
||||
set_default_app_approval_mode_and_reviewer(
|
||||
config,
|
||||
AppToolApproval::Writes,
|
||||
ApprovalsReviewer::User,
|
||||
);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
submit_user_turn(
|
||||
&test,
|
||||
"Use [$calendar](app://calendar) to list events, then create one.",
|
||||
AskForApproval::OnRequest,
|
||||
/*collaboration_mode*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let EventMsg::McpToolCallBegin(read_begin) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::McpToolCallBegin(_))
|
||||
})
|
||||
.await
|
||||
else {
|
||||
unreachable!("event guard guarantees McpToolCallBegin");
|
||||
};
|
||||
assert_eq!(read_begin.call_id, read_call_id);
|
||||
|
||||
let next_route = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::McpToolCallBegin(_) | EventMsg::ElicitationRequest(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
let EventMsg::McpToolCallBegin(write_begin) = next_route else {
|
||||
panic!("read-only app action should not prompt in writes mode");
|
||||
};
|
||||
assert_eq!(write_begin.call_id, write_call_id);
|
||||
|
||||
let EventMsg::ElicitationRequest(request) = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ElicitationRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
})
|
||||
.await
|
||||
else {
|
||||
panic!("write app action should prompt in writes mode");
|
||||
};
|
||||
|
||||
test.codex
|
||||
.submit(Op::ResolveElicitation {
|
||||
server_name: request.server_name,
|
||||
request_id: request.id,
|
||||
decision: ElicitationAction::Accept,
|
||||
content: None,
|
||||
meta: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(responses.requests().len(), 3);
|
||||
recorded_apps_tool_call_by_call_id(&server, read_call_id).await;
|
||||
recorded_apps_tool_call_by_call_id(&server, write_call_id).await;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user