Skip to content
Merged
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
4 changes: 2 additions & 2 deletions codex-rs/analytics/src/analytics_client_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ fn sample_thread_start_response(
cwd: test_path_buf("/tmp").abs(),
runtime_workspace_roots: Vec::new(),
instruction_sources: Vec::new(),
approval_policy: AppServerAskForApproval::OnFailure,
approval_policy: AppServerAskForApproval::OnRequest,
approvals_reviewer: AppServerApprovalsReviewer::User,
sandbox: AppServerSandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand Down Expand Up @@ -284,7 +284,7 @@ fn sample_thread_resume_response_with_source(
cwd: test_path_buf("/tmp").abs(),
runtime_workspace_roots: Vec::new(),
instruction_sources: Vec::new(),
approval_policy: AppServerAskForApproval::OnFailure,
approval_policy: AppServerAskForApproval::OnRequest,
approvals_reviewer: AppServerApprovalsReviewer::User,
sandbox: AppServerSandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand Down
6 changes: 3 additions & 3 deletions codex-rs/analytics/src/client_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ fn sample_thread_start_response() -> ClientResponsePayload {
cwd: test_path_buf("/tmp").abs(),
runtime_workspace_roots: Vec::new(),
instruction_sources: Vec::new(),
approval_policy: AppServerAskForApproval::OnFailure,
approval_policy: AppServerAskForApproval::OnRequest,
approvals_reviewer: AppServerApprovalsReviewer::User,
sandbox: AppServerSandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand All @@ -327,7 +327,7 @@ fn sample_thread_resume_response() -> ClientResponsePayload {
cwd: test_path_buf("/tmp").abs(),
runtime_workspace_roots: Vec::new(),
instruction_sources: Vec::new(),
approval_policy: AppServerAskForApproval::OnFailure,
approval_policy: AppServerAskForApproval::OnRequest,
approvals_reviewer: AppServerApprovalsReviewer::User,
sandbox: AppServerSandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand All @@ -346,7 +346,7 @@ fn sample_thread_fork_response() -> ClientResponsePayload {
cwd: test_path_buf("/tmp").abs(),
runtime_workspace_roots: Vec::new(),
instruction_sources: Vec::new(),
approval_policy: AppServerAskForApproval::OnFailure,
approval_policy: AppServerAskForApproval::OnRequest,
approvals_reviewer: AppServerApprovalsReviewer::User,
sandbox: AppServerSandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions codex-rs/app-server-protocol/src/protocol/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2604,7 +2604,7 @@ mod tests {
"/tmp/AGENTS.md",
)),
],
approval_policy: v2::AskForApproval::OnFailure,
approval_policy: v2::AskForApproval::OnRequest,
approvals_reviewer: v2::ApprovalsReviewer::User,
sandbox: v2::SandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand Down Expand Up @@ -2651,7 +2651,7 @@ mod tests {
"cwd": absolute_path_string("tmp"),
"runtimeWorkspaceRoots": [],
"instructionSources": [absolute_path_string("tmp/AGENTS.md")],
"approvalPolicy": "on-failure",
"approvalPolicy": "on-request",
"approvalsReviewer": "user",
"sandbox": {
"type": "dangerFullAccess"
Expand Down
3 changes: 0 additions & 3 deletions codex-rs/app-server-protocol/src/protocol/v2/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ pub enum AskForApproval {
#[serde(rename = "untrusted")]
#[ts(rename = "untrusted")]
UnlessTrusted,
OnFailure,
OnRequest,
#[experimental("askForApproval.granular")]
Granular {
Expand All @@ -182,7 +181,6 @@ impl AskForApproval {
pub fn to_core(self) -> CoreAskForApproval {
match self {
AskForApproval::UnlessTrusted => CoreAskForApproval::UnlessTrusted,
AskForApproval::OnFailure => CoreAskForApproval::OnFailure,
AskForApproval::OnRequest => CoreAskForApproval::OnRequest,
AskForApproval::Granular {
sandbox_approval,
Expand All @@ -206,7 +204,6 @@ impl From<CoreAskForApproval> for AskForApproval {
fn from(value: CoreAskForApproval) -> Self {
match value {
CoreAskForApproval::UnlessTrusted => AskForApproval::UnlessTrusted,
CoreAskForApproval::OnFailure => AskForApproval::OnFailure,
CoreAskForApproval::OnRequest => AskForApproval::OnRequest,
CoreAskForApproval::Granular(granular_config) => AskForApproval::Granular {
sandbox_approval: granular_config.sandbox_approval,
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/app-server-protocol/src/protocol/v2/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ fn thread_resume_response_round_trips_initial_turns_page() {
cwd: absolute_path("tmp"),
runtime_workspace_roots: Vec::new(),
instruction_sources: Vec::new(),
approval_policy: AskForApproval::OnFailure,
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: ApprovalsReviewer::User,
sandbox: SandboxPolicy::DangerFullAccess,
active_permission_profile: None,
Expand Down Expand Up @@ -3689,7 +3689,7 @@ fn thread_lifecycle_responses_default_missing_optional_fields() {
"modelProvider": "openai",
"serviceTier": null,
"cwd": absolute_path_string("tmp"),
"approvalPolicy": "on-failure",
"approvalPolicy": "on-request",
"approvalsReviewer": "user",
"sandbox": { "type": "dangerFullAccess" },
"reasoningEffort": null
Expand Down
27 changes: 12 additions & 15 deletions codex-rs/codex-mcp/src/connection_manager_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,6 @@ fn tool_with_model_visible_input_schema_leaves_tools_without_file_params_unchang

#[test]
fn elicitation_granular_policy_defaults_to_prompting() {
assert!(!elicitation_is_rejected_by_policy(
AskForApproval::OnFailure
));
assert!(!elicitation_is_rejected_by_policy(
AskForApproval::OnRequest
));
Expand Down Expand Up @@ -789,7 +786,7 @@ async fn list_all_tools_uses_cached_tool_info_snapshot_while_client_is_pending()
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -826,7 +823,7 @@ async fn list_available_server_infos_uses_cache_while_client_is_pending() {
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -865,7 +862,7 @@ async fn list_all_tools_accepts_canonical_namespaced_tool_names() {
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -909,7 +906,7 @@ async fn list_all_tools_applies_legacy_mcp_prefix_by_default() {
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -952,7 +949,7 @@ async fn list_all_tools_blocks_while_client_is_pending_without_cached_tool_info_
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -989,7 +986,7 @@ async fn shutdown_cancels_pending_tool_listing() {
}
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -1025,7 +1022,7 @@ async fn list_all_tools_does_not_block_when_cached_tool_info_snapshot_is_empty()
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -1065,7 +1062,7 @@ async fn list_all_tools_uses_cached_tool_info_snapshot_when_client_startup_fails
))
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -1112,7 +1109,7 @@ async fn list_all_tools_adds_server_metadata_to_cached_tools() {
let pending_client = futures::future::pending::<Result<ManagedClient, StartupOutcomeError>>()
.boxed()
.shared();
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand Down Expand Up @@ -1179,7 +1176,7 @@ fn server_metadata_preserves_tool_approval_policy() {

#[test]
fn host_owned_codex_apps_requires_server_metadata() {
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand All @@ -1192,7 +1189,7 @@ fn host_owned_codex_apps_requires_server_metadata() {

#[test]
fn host_owned_codex_apps_matches_reserved_name_with_server_metadata() {
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let permission_profile = Constrained::allow_any(PermissionProfile::default());
let mut manager = McpConnectionManager::new_uninitialized(
&approval_policy,
Expand All @@ -1214,7 +1211,7 @@ fn host_owned_codex_apps_matches_reserved_name_with_server_metadata() {

#[tokio::test]
async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() {
let approval_policy = Constrained::allow_any(AskForApproval::OnFailure);
let approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let (tx_event, rx_event) = async_channel::unbounded();
drop(rx_event);
let codex_home = tempdir().expect("tempdir");
Expand Down
1 change: 0 additions & 1 deletion codex-rs/codex-mcp/src/elicitation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ impl ElicitationRequestManager {
pub(crate) fn elicitation_is_rejected_by_policy(approval_policy: AskForApproval) -> bool {
match approval_policy {
AskForApproval::Never => true,
AskForApproval::OnFailure => false,
AskForApproval::OnRequest => false,
AskForApproval::UnlessTrusted => false,
AskForApproval::Granular(granular_config) => !granular_config.allows_mcp_elicitations(),
Expand Down
3 changes: 1 addition & 2 deletions codex-rs/codex-mcp/src/mcp/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
mcp_oauth_callback_port: None,
mcp_oauth_callback_url: None,
skill_mcp_dependency_install_enabled: true,
approval_policy: Constrained::allow_any(AskForApproval::OnFailure),
approval_policy: Constrained::allow_any(AskForApproval::OnRequest),
codex_linux_sandbox_exe: None,
use_legacy_landlock: false,
apps_enabled: false,
Expand Down Expand Up @@ -83,7 +83,6 @@ fn mcp_prompt_auto_approval_honors_unrestricted_managed_profiles() {
fn mcp_prompt_auto_approval_honors_approved_tools_in_all_permission_modes() {
for approval_policy in [
AskForApproval::UnlessTrusted,
AskForApproval::OnFailure,
AskForApproval::OnRequest,
AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
Expand Down
11 changes: 0 additions & 11 deletions codex-rs/config/src/config_requirements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2513,17 +2513,6 @@ allowed_approvals_reviewers = ["user"]
.can_set(&AskForApproval::UnlessTrusted)
.is_ok()
);
assert_eq!(
requirements
.approval_policy
.can_set(&AskForApproval::OnFailure),
Err(ConstraintError::InvalidValue {
field_name: "approval_policy",
candidate: "OnFailure".into(),
allowed: "[UnlessTrusted, OnRequest]".into(),
requirement_source: RequirementSource::Unknown,
})
);
assert!(
requirements
.approval_policy
Expand Down
Loading
Loading