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
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,10 @@ impl ThreadMonitorRequestProcessor {
let writer_fence = writer.monitor_writer_fence().to_string();
let before = writer.config_snapshot().await;
let approval_cwd = monitor_approval_cwd(&before.cwd, cwd)?;
self.request_monitor_command_approval(request_id, thread_id, command, approval_cwd)
.await?;
if monitor_command_requires_approval(before.approval_policy) {
self.request_monitor_command_approval(request_id, thread_id, command, approval_cwd)
.await?;
}

let current_writer = self.current_monitor_writer(thread_id).await?;
let after = current_writer.config_snapshot().await;
Expand Down Expand Up @@ -749,6 +751,10 @@ fn monitor_command_approval_granted(decision: &CommandExecutionApprovalDecision)
matches!(decision, CommandExecutionApprovalDecision::Accept)
}

fn monitor_command_requires_approval(policy: codex_protocol::protocol::AskForApproval) -> bool {
!matches!(policy, codex_protocol::protocol::AskForApproval::Never)
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -768,4 +774,14 @@ mod tests {
&CommandExecutionApprovalDecision::Cancel
));
}

#[test]
fn monitor_command_approval_respects_persisted_policy() {
assert!(!monitor_command_requires_approval(
codex_protocol::protocol::AskForApproval::Never
));
assert!(monitor_command_requires_approval(
codex_protocol::protocol::AskForApproval::OnRequest
));
}
}
59 changes: 36 additions & 23 deletions codex-rs/core/src/tools/handlers/monitor_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::tools::handlers::parse_arguments;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use codex_protocol::ThreadId;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
Expand Down Expand Up @@ -431,29 +432,31 @@ async fn authorize_monitor_command(
) -> Result<codex_state::ThreadMonitorAuthorization, FunctionCallError> {
let before = session.thread_config_snapshot().await;
let approval_cwd = monitor_approval_cwd(&before.cwd, cwd)?;
let decision = session
.request_command_approval(
turn,
call_id.to_string(),
/*approval_id*/ None,
crate::exec::persistent_shell_command_args(command),
approval_cwd,
Some(
"Authorize this persistent monitor command. It may run in the background until stopped."
.to_string(),
),
/*network_approval_context*/ None,
/*proposed_execpolicy_amendment*/ None,
/*additional_permissions*/ None,
Some(vec![
ReviewDecision::Approved,
ReviewDecision::Denied,
ReviewDecision::Abort,
]),
)
.await;
if !monitor_command_approval_granted(&decision) {
return Err(model_error("monitor command approval was not granted"));
if monitor_command_requires_approval(before.approval_policy) {
let decision = session
.request_command_approval(
turn,
call_id.to_string(),
/*approval_id*/ None,
crate::exec::persistent_shell_command_args(command),
approval_cwd,
Some(
"Authorize this persistent monitor command. It may run in the background until stopped."
.to_string(),
),
/*network_approval_context*/ None,
/*proposed_execpolicy_amendment*/ None,
/*additional_permissions*/ None,
Some(vec![
ReviewDecision::Approved,
ReviewDecision::Denied,
ReviewDecision::Abort,
]),
)
.await;
if !monitor_command_approval_granted(&decision) {
return Err(model_error("monitor command approval was not granted"));
}
}

let after = session.thread_config_snapshot().await;
Expand Down Expand Up @@ -487,6 +490,10 @@ fn monitor_command_approval_granted(decision: &ReviewDecision) -> bool {
matches!(decision, ReviewDecision::Approved)
}

fn monitor_command_requires_approval(policy: AskForApproval) -> bool {
!matches!(policy, AskForApproval::Never)
}

async fn delete_monitor(
state_db: Arc<codex_state::StateRuntime>,
thread_id: ThreadId,
Expand Down Expand Up @@ -1038,4 +1045,10 @@ mod tests {
assert!(!monitor_command_approval_granted(&ReviewDecision::Denied));
assert!(!monitor_command_approval_granted(&ReviewDecision::Abort));
}

#[test]
fn monitor_command_approval_respects_persisted_policy() {
assert!(!monitor_command_requires_approval(AskForApproval::Never));
assert!(monitor_command_requires_approval(AskForApproval::OnRequest));
}
}
Loading