From 5c00e1596a3adf0fd0683a2962475813cd38159f Mon Sep 17 00:00:00 2001 From: jimmyfraiture Date: Tue, 30 Sep 2025 13:28:25 +0100 Subject: [PATCH] Restore otel --- codex-rs/core/src/codex.rs | 9 ++++--- codex-rs/core/src/executor/runner.rs | 34 ++++++++++++++--------- codex-rs/core/src/executor/sandbox.rs | 39 +++++++++++++++++++++++---- codex-rs/core/src/safety.rs | 5 ++-- 4 files changed, 64 insertions(+), 23 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2fb6bbfc17..3fc0da7829 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1,7 +1,6 @@ use std::borrow::Cow; use std::collections::HashMap; use std::fmt::Debug; -use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicU64; @@ -125,7 +124,6 @@ use crate::user_instructions::UserInstructions; use crate::user_notification::UserNotification; use crate::util::backoff; use codex_otel::otel_event_manager::OtelEventManager; -use codex_otel::otel_event_manager::ToolDecisionSource; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::custom_prompts::CustomPrompt; @@ -833,6 +831,7 @@ impl Session { command_for_display, cwd, apply_patch, + .. } = exec_command_context; let msg = match apply_patch { Some(ApplyPatchCommandContext { @@ -946,7 +945,7 @@ impl Session { let result = self .services .executor - .run(request, self, approval_policy, &sub_id, &call_id) + .run(request, self, approval_policy, &context) .await; let normalized = normalize_exec_result(&result); @@ -1079,6 +1078,8 @@ pub(crate) struct ExecCommandContext { pub(crate) command_for_display: Vec, pub(crate) cwd: PathBuf, pub(crate) apply_patch: Option, + pub(crate) tool_name: String, + pub(crate) otel_event_manager: OtelEventManager, } #[derive(Clone, Debug)] @@ -2662,6 +2663,8 @@ async fn handle_container_exec_with_params( changes: convert_apply_patch_to_protocol(action), }, ), + tool_name: tool_name.to_string(), + otel_event_manager, }; let mode = match apply_patch_exec { diff --git a/codex-rs/core/src/executor/runner.rs b/codex-rs/core/src/executor/runner.rs index 86caf313a8..ede9ed49e7 100644 --- a/codex-rs/core/src/executor/runner.rs +++ b/codex-rs/core/src/executor/runner.rs @@ -6,6 +6,7 @@ use std::time::Duration; use super::backends::ExecutionMode; use super::backends::backend_for_mode; use super::cache::ApprovalCache; +use crate::codex::ExecCommandContext; use crate::codex::Session; use crate::error::CodexErr; use crate::error::SandboxErr; @@ -23,6 +24,7 @@ use crate::protocol::AskForApproval; use crate::protocol::ReviewDecision; use crate::protocol::SandboxPolicy; use crate::shell; +use codex_otel::otel_event_manager::ToolDecisionSource; #[derive(Clone, Debug)] pub(crate) struct ExecutorConfig { @@ -77,8 +79,7 @@ impl Executor { mut request: ExecutionRequest, session: &Session, approval_policy: AskForApproval, - sub_id: &str, - call_id: &str, + context: &ExecCommandContext, ) -> Result { if matches!(request.mode, ExecutionMode::Shell) { request.params = @@ -110,8 +111,9 @@ impl Executor { self.approval_cache.snapshot(), &config, session, - sub_id, - call_id, + &context.sub_id, + &context.call_id, + &context.otel_event_manager, ) .await?; if sandbox_decision.record_session_approval { @@ -140,8 +142,7 @@ impl Executor { &request, &config, session, - sub_id, - call_id, + context, stdout_stream.clone(), error, ) @@ -161,37 +162,44 @@ impl Executor { /// Fallback path invoked when a sandboxed run is denied so the user can /// approve rerunning without isolation. - #[allow(clippy::too_many_arguments)] async fn retry_without_sandbox( &self, request: &ExecutionRequest, config: &ExecutorConfig, session: &Session, - sub_id: &str, - call_id: &str, + context: &ExecCommandContext, stdout_stream: Option, sandbox_error: SandboxErr, ) -> Result { session - .notify_background_event(sub_id, format!("Execution failed: {sandbox_error}")) + .notify_background_event( + &context.sub_id, + format!("Execution failed: {sandbox_error}"), + ) .await; let decision = session .request_command_approval( - sub_id.to_string(), - call_id.to_string(), + context.sub_id.to_string(), + context.call_id.to_string(), request.approval_command.clone(), request.params.cwd.clone(), Some("command failed; retry without sandbox?".to_string()), ) .await; + context.otel_event_manager.tool_decision( + &context.tool_name, + &context.call_id, + decision, + ToolDecisionSource::User, + ); match decision { ReviewDecision::Approved | ReviewDecision::ApprovedForSession => { if matches!(decision, ReviewDecision::ApprovedForSession) { self.approval_cache.insert(request.approval_command.clone()); } session - .notify_background_event(sub_id, "retrying command without sandbox") + .notify_background_event(&context.sub_id, "retrying command without sandbox") .await; let retry_output = self diff --git a/codex-rs/core/src/executor/sandbox.rs b/codex-rs/core/src/executor/sandbox.rs index 3d3eaceaee..4c619c3e6e 100644 --- a/codex-rs/core/src/executor/sandbox.rs +++ b/codex-rs/core/src/executor/sandbox.rs @@ -8,6 +8,8 @@ use crate::executor::errors::ExecError; use crate::safety::SafetyCheck; use crate::safety::assess_command_safety; use crate::safety::assess_patch_safety; +use codex_otel::otel_event_manager::OtelEventManager; +use codex_otel::otel_event_manager::ToolDecisionSource; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use std::collections::HashSet; @@ -50,6 +52,7 @@ fn should_escalate_on_failure(approval: AskForApproval, sandbox: SandboxType) -> /// Determines how a command should be sandboxed, prompting the user when /// policy requires explicit approval. +#[warn(clippy::too_many_arguments)] pub async fn select_sandbox( request: &ExecutionRequest, approval_policy: AskForApproval, @@ -58,6 +61,7 @@ pub async fn select_sandbox( session: &Session, sub_id: &str, call_id: &str, + otel_event_manager: &OtelEventManager, ) -> Result { match &request.mode { ExecutionMode::Shell => { @@ -69,6 +73,7 @@ pub async fn select_sandbox( session, sub_id, call_id, + otel_event_manager, ) .await } @@ -78,6 +83,7 @@ pub async fn select_sandbox( } } +#[warn(clippy::too_many_arguments)] async fn select_shell_sandbox( request: &ExecutionRequest, approval_policy: AskForApproval, @@ -86,6 +92,7 @@ async fn select_shell_sandbox( session: &Session, sub_id: &str, call_id: &str, + otel_event_manager: &OtelEventManager, ) -> Result { let command_for_safety = if request.approval_command.is_empty() { request.params.command.clone() @@ -113,6 +120,12 @@ async fn select_shell_sandbox( if user_explicitly_approved { decision.record_session_approval = true; } + let (decision_for_event, source) = if user_explicitly_approved { + (ReviewDecision::ApprovedForSession, ToolDecisionSource::User) + } else { + (ReviewDecision::Approved, ToolDecisionSource::Config) + }; + otel_event_manager.tool_decision("local_shell", call_id, decision_for_event, source); Ok(decision) } SafetyCheck::AskUser => { @@ -126,6 +139,12 @@ async fn select_shell_sandbox( ) .await; + otel_event_manager.tool_decision( + "local_shell", + call_id, + decision, + ToolDecisionSource::User, + ); match decision { ReviewDecision::Approved => Ok(SandboxDecision::user_override(false)), ReviewDecision::ApprovedForSession => Ok(SandboxDecision::user_override(true)), @@ -180,7 +199,7 @@ mod tests { #[tokio::test] async fn select_apply_patch_user_override_when_explicit() { - let (session, _ctx) = make_session_and_context(); + let (session, ctx) = make_session_and_context(); let tmp = tempfile::tempdir().expect("tmp"); let p = tmp.path().join("a.txt"); let action = ApplyPatchAction::new_add_for_test(&p, "hello".to_string()); @@ -203,6 +222,7 @@ mod tests { stdout_stream: None, use_shell_profile: false, }; + let otel_event_manager = ctx.client.get_otel_event_manager(); let decision = select_sandbox( &request, AskForApproval::OnRequest, @@ -211,6 +231,7 @@ mod tests { &session, "sub", "call", + &otel_event_manager, ) .await .expect("ok"); @@ -221,7 +242,7 @@ mod tests { #[tokio::test] async fn select_apply_patch_autoapprove_in_danger() { - let (session, _ctx) = make_session_and_context(); + let (session, ctx) = make_session_and_context(); let tmp = tempfile::tempdir().expect("tmp"); let p = tmp.path().join("a.txt"); let action = ApplyPatchAction::new_add_for_test(&p, "hello".to_string()); @@ -244,6 +265,7 @@ mod tests { stdout_stream: None, use_shell_profile: false, }; + let otel_event_manager = ctx.client.get_otel_event_manager(); let decision = select_sandbox( &request, AskForApproval::OnRequest, @@ -252,6 +274,7 @@ mod tests { &session, "sub", "call", + &otel_event_manager, ) .await .expect("ok"); @@ -263,7 +286,7 @@ mod tests { #[tokio::test] async fn select_apply_patch_requires_approval_on_unless_trusted() { - let (session, _ctx) = make_session_and_context(); + let (session, ctx) = make_session_and_context(); let tempdir = tempfile::tempdir().expect("tmpdir"); let p = tempdir.path().join("a.txt"); let action = ApplyPatchAction::new_add_for_test(&p, "hello".to_string()); @@ -286,6 +309,7 @@ mod tests { stdout_stream: None, use_shell_profile: false, }; + let otel_event_manager = ctx.client.get_otel_event_manager(); let result = select_sandbox( &request, AskForApproval::UnlessTrusted, @@ -294,6 +318,7 @@ mod tests { &session, "sub", "call", + &otel_event_manager, ) .await; match result { @@ -307,7 +332,7 @@ mod tests { #[tokio::test] async fn select_shell_autoapprove_in_danger_mode() { - let (session, _ctx) = make_session_and_context(); + let (session, ctx) = make_session_and_context(); let cfg = ExecutorConfig::new(SandboxPolicy::DangerFullAccess, std::env::temp_dir(), None); let request = ExecutionRequest { params: ExecParams { @@ -323,6 +348,7 @@ mod tests { stdout_stream: None, use_shell_profile: false, }; + let otel_event_manager = ctx.client.get_otel_event_manager(); let decision = select_sandbox( &request, AskForApproval::OnRequest, @@ -331,6 +357,7 @@ mod tests { &session, "sub", "call", + &otel_event_manager, ) .await .expect("ok"); @@ -341,7 +368,7 @@ mod tests { #[cfg(any(target_os = "macos", target_os = "linux"))] #[tokio::test] async fn select_shell_escalates_on_failure_with_platform_sandbox() { - let (session, _ctx) = make_session_and_context(); + let (session, ctx) = make_session_and_context(); let cfg = ExecutorConfig::new(SandboxPolicy::ReadOnly, std::env::temp_dir(), None); let request = ExecutionRequest { params: ExecParams { @@ -358,6 +385,7 @@ mod tests { stdout_stream: None, use_shell_profile: false, }; + let otel_event_manager = ctx.client.get_otel_event_manager(); let decision = select_sandbox( &request, AskForApproval::OnFailure, @@ -366,6 +394,7 @@ mod tests { &session, "sub", "call", + &otel_event_manager, ) .await .expect("ok"); diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index b976ae4a4c..0ed0f929ff 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -125,9 +125,10 @@ pub fn assess_command_safety( // the session _because_ they know it needs to run outside a sandbox. if is_known_safe_command(command) || approved.contains(command) { + let user_explicitly_approved = approved.contains(command); return SafetyCheck::AutoApprove { sandbox_type: SandboxType::None, - user_explicitly_approved: false, + user_explicitly_approved, }; } @@ -380,7 +381,7 @@ mod tests { safety_check, SafetyCheck::AutoApprove { sandbox_type: SandboxType::None, - user_explicitly_approved: false, + user_explicitly_approved: true, } ); }