feat: separate protocol vs internal versions of AskForApproval

This commit is contained in:
Michael Bolin
2026-02-17 16:57:41 -08:00
parent 31cbebd3c2
commit 5946ffb1c5
9 changed files with 155 additions and 45 deletions
@@ -830,7 +830,6 @@ mod tests {
use codex_protocol::ThreadId;
use codex_protocol::account::PlanType;
use codex_protocol::parse_command::ParsedCommand;
use codex_protocol::protocol::AskForApproval;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::path::PathBuf;
@@ -844,7 +843,7 @@ mod tests {
model_provider: None,
profile: None,
cwd: None,
approval_policy: Some(AskForApproval::OnRequest),
approval_policy: Some(v1::AskForApprovalProtocolV1::OnRequest),
sandbox: None,
config: None,
base_instructions: None,
@@ -9,7 +9,7 @@ use codex_protocol::config_types::Verbosity;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::parse_command::ParsedCommand;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::AskForApproval as CoreAskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::ReviewDecision;
@@ -71,7 +71,7 @@ pub struct NewConversationParams {
pub model_provider: Option<String>,
pub profile: Option<String>,
pub cwd: Option<String>,
pub approval_policy: Option<AskForApproval>,
pub approval_policy: Option<AskForApprovalProtocolV1>,
pub sandbox: Option<SandboxMode>,
pub config: Option<HashMap<String, serde_json::Value>>,
pub base_instructions: Option<String>,
@@ -358,7 +358,7 @@ pub struct SetDefaultModelResponse {}
#[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
pub struct UserSavedConfig {
pub approval_policy: Option<AskForApproval>,
pub approval_policy: Option<AskForApprovalProtocolV1>,
pub sandbox_mode: Option<SandboxMode>,
pub sandbox_settings: Option<SandboxSettings>,
pub forced_chatgpt_workspace_id: Option<String>,
@@ -377,7 +377,7 @@ pub struct UserSavedConfig {
pub struct Profile {
pub model: Option<String>,
pub model_provider: Option<String>,
pub approval_policy: Option<AskForApproval>,
pub approval_policy: Option<AskForApprovalProtocolV1>,
pub model_reasoning_effort: Option<ReasoningEffort>,
pub model_reasoning_summary: Option<ReasoningSummary>,
pub model_verbosity: Option<Verbosity>,
@@ -414,7 +414,7 @@ pub struct SendUserTurnParams {
pub conversation_id: ThreadId,
pub items: Vec<InputItem>,
pub cwd: PathBuf,
pub approval_policy: AskForApproval,
pub approval_policy: AskForApprovalProtocolV1,
pub sandbox_policy: SandboxPolicy,
pub model: String,
pub effort: Option<ReasoningEffort>,
@@ -529,6 +529,53 @@ impl From<V1TextElement> for CoreTextElement {
}
}
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "kebab-case")]
pub enum AskForApprovalProtocolV1 {
/// Under this policy, only "known safe" commands—those matching
/// `is_safe_command()` and performing read-only file operations—are
/// auto-approved. Everything else will ask the user to approve.
#[serde(rename = "untrusted")]
UnlessTrusted,
/// DEPRECATED: *All* commands are auto-approved, but they are expected to
/// run inside a sandbox where network access is disabled and writes are
/// confined to a specific set of paths. If the command fails, it will be
/// escalated to the user to approve execution without a sandbox.
/// Prefer `on-request` for interactive runs or `never` for non-interactive
/// runs.
OnFailure,
/// The model decides when to ask the user for approval.
OnRequest,
/// Never ask the user to approve commands. Failures are immediately returned
/// to the model, and never escalated to the user for approval.
Never,
}
impl AskForApprovalProtocolV1 {
pub fn to_core(self) -> CoreAskForApproval {
match self {
AskForApprovalProtocolV1::UnlessTrusted => CoreAskForApproval::UnlessTrusted,
AskForApprovalProtocolV1::OnFailure => CoreAskForApproval::OnFailure,
AskForApprovalProtocolV1::OnRequest => CoreAskForApproval::OnRequest,
AskForApprovalProtocolV1::Never => CoreAskForApproval::Never,
}
}
}
impl From<CoreAskForApproval> for AskForApprovalProtocolV1 {
fn from(value: CoreAskForApproval) -> Self {
match value {
CoreAskForApproval::UnlessTrusted => AskForApprovalProtocolV1::UnlessTrusted,
CoreAskForApproval::OnFailure => AskForApprovalProtocolV1::OnFailure,
CoreAskForApproval::OnRequest => AskForApprovalProtocolV1::OnRequest,
CoreAskForApproval::Never => AskForApprovalProtocolV1::Never,
}
}
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
/// Deprecated in favor of AccountLoginCompletedNotification.
@@ -24,7 +24,7 @@ use codex_app_server_protocol::AppsListParams;
use codex_app_server_protocol::AppsListResponse;
use codex_app_server_protocol::ArchiveConversationParams;
use codex_app_server_protocol::ArchiveConversationResponse;
use codex_app_server_protocol::AskForApproval;
use codex_app_server_protocol::AskForApprovalProtocolV1 as V1AskForApproval;
use codex_app_server_protocol::AuthMode;
use codex_app_server_protocol::AuthStatusChangeNotification;
use codex_app_server_protocol::CancelLoginAccountParams;
@@ -229,6 +229,7 @@ use codex_protocol::dynamic_tools::DynamicToolSpec as CoreDynamicToolSpec;
use codex_protocol::items::TurnItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::GitInfo as CoreGitInfo;
use codex_protocol::protocol::McpAuthStatus as CoreMcpAuthStatus;
use codex_protocol::protocol::McpServerRefreshConfig;
@@ -1831,7 +1832,7 @@ impl CodexMessageProcessor {
model,
config_profile: profile,
cwd: cwd.clone().map(PathBuf::from),
approval_policy,
approval_policy: approval_policy.map(V1AskForApproval::to_core),
sandbox_mode,
model_provider,
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
@@ -1941,7 +1942,7 @@ impl CodexMessageProcessor {
model,
model_provider,
cwd,
approval_policy,
approval_policy.map(codex_app_server_protocol::AskForApproval::to_core),
sandbox,
base_instructions,
developer_instructions,
@@ -2064,7 +2065,7 @@ impl CodexMessageProcessor {
model: Option<String>,
model_provider: Option<String>,
cwd: Option<String>,
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
approval_policy: Option<AskForApproval>,
sandbox: Option<SandboxMode>,
base_instructions: Option<String>,
developer_instructions: Option<String>,
@@ -2074,8 +2075,7 @@ impl CodexMessageProcessor {
model,
model_provider,
cwd: cwd.map(PathBuf::from),
approval_policy: approval_policy
.map(codex_app_server_protocol::AskForApproval::to_core),
approval_policy,
sandbox_mode: sandbox.map(SandboxMode::to_core),
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
base_instructions,
@@ -2781,7 +2781,7 @@ impl CodexMessageProcessor {
model,
model_provider,
cwd,
approval_policy,
approval_policy.map(codex_app_server_protocol::AskForApproval::to_core),
sandbox,
base_instructions,
developer_instructions,
@@ -3248,7 +3248,7 @@ impl CodexMessageProcessor {
model,
model_provider,
cwd,
approval_policy,
approval_policy.map(codex_app_server_protocol::AskForApproval::to_core),
sandbox,
base_instructions,
developer_instructions,
@@ -4198,7 +4198,7 @@ impl CodexMessageProcessor {
model,
config_profile: profile,
cwd: cwd.map(PathBuf::from),
approval_policy,
approval_policy: approval_policy.map(V1AskForApproval::to_core),
sandbox_mode,
model_provider,
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
@@ -4393,7 +4393,7 @@ impl CodexMessageProcessor {
model,
config_profile: profile,
cwd: cwd.map(PathBuf::from),
approval_policy,
approval_policy: approval_policy.map(V1AskForApproval::to_core),
sandbox_mode,
model_provider,
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
@@ -4767,7 +4767,7 @@ impl CodexMessageProcessor {
.submit(Op::UserTurn {
items: mapped_items,
cwd,
approval_policy,
approval_policy: approval_policy.to_core(),
sandbox_policy,
model,
effort,
@@ -5252,7 +5252,9 @@ impl CodexMessageProcessor {
let _ = thread
.submit(Op::OverrideTurnContext {
cwd: params.cwd,
approval_policy: params.approval_policy.map(AskForApproval::to_core),
approval_policy: params
.approval_policy
.map(codex_app_server_protocol::AskForApproval::to_core),
sandbox_policy: params.sandbox_policy.map(|p| p.to_core()),
windows_sandbox_level: None,
model: params.model,
@@ -6097,8 +6099,8 @@ fn collect_resume_override_mismatches(
}
}
if let Some(requested_approval) = request.approval_policy.as_ref() {
let active_approval: AskForApproval = config_snapshot.approval_policy.into();
if requested_approval != &active_approval {
let active_approval: AskForApproval = config_snapshot.approval_policy;
if *requested_approval != active_approval.into() {
mismatch_details.push(format!(
"approval_policy requested={requested_approval:?} active={active_approval:?}"
));
@@ -7,6 +7,7 @@ use app_test_support::format_with_current_shell;
use app_test_support::to_response;
use codex_app_server_protocol::AddConversationListenerParams;
use codex_app_server_protocol::AddConversationSubscriptionResponse;
use codex_app_server_protocol::AskForApprovalProtocolV1;
use codex_app_server_protocol::ExecCommandApprovalParams;
use codex_app_server_protocol::InputItem;
use codex_app_server_protocol::JSONRPCNotification;
@@ -21,7 +22,6 @@ use codex_app_server_protocol::SendUserMessageResponse;
use codex_app_server_protocol::SendUserTurnParams;
use codex_app_server_protocol::SendUserTurnResponse;
use codex_app_server_protocol::ServerRequest;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::SandboxPolicy;
use codex_core::protocol_config_types::ReasoningSummary;
use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
@@ -331,7 +331,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> {
text_elements: Vec::new(),
}],
cwd: working_directory.clone(),
approval_policy: AskForApproval::Never,
approval_policy: AskForApprovalProtocolV1::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
model: "mock-model".to_string(),
effort: Some(ReasoningEffort::Medium),
@@ -405,7 +405,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<(
let new_conv_id = mcp
.send_new_conversation_request(NewConversationParams {
cwd: Some(first_cwd.to_string_lossy().into_owned()),
approval_policy: Some(AskForApproval::Never),
approval_policy: Some(AskForApprovalProtocolV1::Never),
sandbox: Some(SandboxMode::WorkspaceWrite),
..Default::default()
})
@@ -441,7 +441,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<(
text_elements: Vec::new(),
}],
cwd: first_cwd.clone(),
approval_policy: AskForApproval::Never,
approval_policy: AskForApprovalProtocolV1::Never,
sandbox_policy: SandboxPolicy::WorkspaceWrite {
writable_roots: vec![first_cwd.try_into()?],
read_only_access: Default::default(),
@@ -475,7 +475,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<(
text_elements: Vec::new(),
}],
cwd: second_cwd.clone(),
approval_policy: AskForApproval::Never,
approval_policy: AskForApprovalProtocolV1::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: model.clone(),
effort: Some(ReasoningEffort::Medium),
+3 -3
View File
@@ -2,6 +2,7 @@ use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::test_tmp_path;
use app_test_support::to_response;
use codex_app_server_protocol::AskForApprovalProtocolV1;
use codex_app_server_protocol::GetUserSavedConfigResponse;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::Profile;
@@ -9,7 +10,6 @@ use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxSettings;
use codex_app_server_protocol::Tools;
use codex_app_server_protocol::UserSavedConfig;
use codex_core::protocol::AskForApproval;
use codex_protocol::config_types::ForcedLoginMethod;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::SandboxMode;
@@ -83,7 +83,7 @@ async fn get_config_toml_parses_all_fields() -> Result<()> {
let writable_root = test_tmp_path();
let expected = GetUserSavedConfigResponse {
config: UserSavedConfig {
approval_policy: Some(AskForApproval::OnRequest),
approval_policy: Some(AskForApprovalProtocolV1::OnRequest),
sandbox_mode: Some(SandboxMode::WorkspaceWrite),
sandbox_settings: Some(SandboxSettings {
writable_roots: vec![writable_root],
@@ -106,7 +106,7 @@ async fn get_config_toml_parses_all_fields() -> Result<()> {
"test".into(),
Profile {
model: Some("gpt-4o".into()),
approval_policy: Some(AskForApproval::OnRequest),
approval_policy: Some(AskForApprovalProtocolV1::OnRequest),
model_reasoning_effort: Some(ReasoningEffort::High),
model_reasoning_summary: Some(ReasoningSummary::Detailed),
model_verbosity: Some(Verbosity::Medium),
@@ -2,6 +2,7 @@ use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::to_response;
use codex_app_server_protocol::AddConversationListenerParams;
use codex_app_server_protocol::AskForApprovalProtocolV1;
use codex_app_server_protocol::InputItem;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::NewConversationParams;
@@ -9,7 +10,6 @@ use codex_app_server_protocol::NewConversationResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SendUserTurnParams;
use codex_app_server_protocol::SendUserTurnResponse;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::SandboxPolicy;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::openai_models::ReasoningEffort;
@@ -83,7 +83,7 @@ async fn send_user_turn_accepts_output_schema_v1() -> Result<()> {
text_elements: Vec::new(),
}],
cwd: codex_home.path().to_path_buf(),
approval_policy: AskForApproval::Never,
approval_policy: AskForApprovalProtocolV1::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
model: "mock-model".to_string(),
effort: Some(ReasoningEffort::Medium),
@@ -185,7 +185,7 @@ async fn send_user_turn_output_schema_is_per_turn_v1() -> Result<()> {
text_elements: Vec::new(),
}],
cwd: codex_home.path().to_path_buf(),
approval_policy: AskForApproval::Never,
approval_policy: AskForApprovalProtocolV1::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
model: "mock-model".to_string(),
effort: Some(ReasoningEffort::Medium),
@@ -233,7 +233,7 @@ async fn send_user_turn_output_schema_is_per_turn_v1() -> Result<()> {
text_elements: Vec::new(),
}],
cwd: codex_home.path().to_path_buf(),
approval_policy: AskForApproval::Never,
approval_policy: AskForApprovalProtocolV1::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
model: "mock-model".to_string(),
effort: Some(ReasoningEffort::Medium),
+64 -4
View File
@@ -893,7 +893,7 @@ pub struct ConfigToml {
pub model_auto_compact_token_limit: Option<i64>,
/// Default approval policy for executing commands.
pub approval_policy: Option<AskForApproval>,
pub approval_policy: Option<AskForApprovalToml>,
#[serde(default)]
pub shell_environment_policy: ShellEnvironmentPolicyToml,
@@ -1113,7 +1113,9 @@ impl From<ConfigToml> for UserSavedConfig {
.collect();
Self {
approval_policy: config_toml.approval_policy,
approval_policy: config_toml
.approval_policy
.map(|approval_policy| Into::into(approval_policy.to_core())),
sandbox_mode: config_toml.sandbox_mode,
sandbox_settings: config_toml.sandbox_workspace_write.map(From::from),
forced_chatgpt_workspace_id: config_toml.forced_chatgpt_workspace_id,
@@ -1129,6 +1131,60 @@ impl From<ConfigToml> for UserSavedConfig {
}
}
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "kebab-case")]
pub enum AskForApprovalToml {
/// Under this policy, only "known safe" commands—those matching
/// `is_safe_command()` and performing read-only file operations—are
/// auto-approved. Everything else will ask the user to approve.
#[serde(rename = "untrusted")]
UnlessTrusted,
/// DEPRECATED: *All* commands are auto-approved, but they are expected to
/// run inside a sandbox where network access is disabled and writes are
/// confined to a specific set of paths. If the command fails, it will be
/// escalated to the user to approve execution without a sandbox.
/// Prefer `OnRequest` for interactive runs or `Never` for non-interactive
/// runs.
OnFailure,
/// The model decides when to ask the user for approval.
#[default]
OnRequest,
/// Never ask the user to approve commands. Failures are immediately returned
/// to the model, and never escalated to the user for approval.
Never,
}
impl AskForApprovalToml {
pub fn to_core(self) -> AskForApproval {
match self {
AskForApprovalToml::UnlessTrusted => AskForApproval::UnlessTrusted,
AskForApprovalToml::OnFailure => AskForApproval::OnFailure,
AskForApprovalToml::OnRequest => AskForApproval::OnRequest,
AskForApprovalToml::Never => AskForApproval::Never,
}
}
}
impl From<AskForApproval> for AskForApprovalToml {
fn from(value: AskForApproval) -> Self {
match value {
AskForApproval::UnlessTrusted => AskForApprovalToml::UnlessTrusted,
AskForApproval::OnFailure => AskForApprovalToml::OnFailure,
AskForApproval::OnRequest => AskForApprovalToml::OnRequest,
AskForApproval::Never => AskForApprovalToml::Never,
}
}
}
impl From<AskForApprovalToml> for AskForApproval {
fn from(value: AskForApprovalToml) -> Self {
value.to_core()
}
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct ProjectConfig {
@@ -1570,8 +1626,12 @@ impl Config {
|| config_profile.approval_policy.is_some()
|| cfg.approval_policy.is_some();
let mut approval_policy = approval_policy_override
.or(config_profile.approval_policy)
.or(cfg.approval_policy)
.or_else(|| {
config_profile
.approval_policy
.map(AskForApprovalToml::to_core)
})
.or_else(|| cfg.approval_policy.map(AskForApprovalToml::to_core))
.unwrap_or_else(|| {
if active_project.is_trusted() {
AskForApproval::OnRequest
+5 -3
View File
@@ -3,9 +3,9 @@ use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
use crate::config::AskForApprovalToml;
use crate::config::types::Personality;
use crate::config::types::WindowsToml;
use crate::protocol::AskForApproval;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::Verbosity;
@@ -21,7 +21,7 @@ pub struct ConfigProfile {
/// The key in the `model_providers` map identifying the
/// [`ModelProviderInfo`] to use.
pub model_provider: Option<String>,
pub approval_policy: Option<AskForApproval>,
pub approval_policy: Option<AskForApprovalToml>,
pub sandbox_mode: Option<SandboxMode>,
pub model_reasoning_effort: Option<ReasoningEffort>,
pub model_reasoning_summary: Option<ReasoningSummary>,
@@ -57,7 +57,9 @@ impl From<ConfigProfile> for codex_app_server_protocol::Profile {
Self {
model: config_profile.model,
model_provider: config_profile.model_provider,
approval_policy: config_profile.approval_policy,
approval_policy: config_profile
.approval_policy
.map(|approval_policy| approval_policy.to_core().into()),
model_reasoning_effort: config_profile.model_reasoning_effort,
model_reasoning_summary: config_profile.model_reasoning_summary,
model_verbosity: config_profile.model_verbosity,