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
86 changes: 81 additions & 5 deletions codex-rs/core/src/tools/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use codex_protocol::protocol::NetworkPolicyRuleAction;
use codex_protocol::protocol::ReviewDecision;
use codex_sandboxing::SandboxManager;
use codex_sandboxing::SandboxType;
use std::time::Instant;

pub(crate) struct ToolOrchestrator {
sandbox: SandboxManager,
Expand Down Expand Up @@ -256,6 +257,7 @@ impl ToolOrchestrator {
network_denial_cancellation_token: None,
};

let initial_attempt_start = Instant::now();
let (first_result, first_deferred_network_approval) = Self::run_attempt(
tool,
req,
Expand All @@ -264,6 +266,7 @@ impl ToolOrchestrator {
managed_network_active,
)
.await;
let initial_duration = initial_attempt_start.elapsed();
match first_result {
Ok(out) => {
// We have a successful initial result
Expand All @@ -284,12 +287,26 @@ impl ToolOrchestrator {
None
};
if network_policy_decision.is_some() && network_approval_context.is_none() {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
"denied",
initial_duration,
/*escalated_duration*/ None,
);
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
network_policy_decision,
})));
}
if !tool.escalate_on_failure() {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
"denied",
initial_duration,
/*escalated_duration*/ None,
);
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
network_policy_decision,
Expand All @@ -312,13 +329,27 @@ impl ToolOrchestrator {
ExecApprovalRequirement::NeedsApproval { .. }
);
if !allow_on_request_network_prompt {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
"denied",
initial_duration,
/*escalated_duration*/ None,
);
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
network_policy_decision,
})));
}
}
if !unsandboxed_allowed && network_approval_context.is_none() {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
"denied",
initial_duration,
/*escalated_duration*/ None,
);
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
network_policy_decision,
Expand Down Expand Up @@ -400,15 +431,51 @@ impl ToolOrchestrator {
};

// Second attempt.
let escalated_attempt_start = Instant::now();
let (retry_result, retry_deferred_network_approval) =
Self::run_attempt(tool, req, tool_ctx, &retry_attempt, managed_network_active)
.await;
retry_result.map(|output| OrchestratorRunResult {
output,
deferred_network_approval: retry_deferred_network_approval,
})
let escalated_duration = escalated_attempt_start.elapsed();
match retry_result {
Ok(output) => {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
"escalated",
initial_duration,
Some(escalated_duration),
);
Ok(OrchestratorRunResult {
output,
deferred_network_approval: retry_deferred_network_approval,
})
}
Err(err) => {
if let Some(outcome) = sandbox_outcome_from_tool_error(&err) {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
outcome,
initial_duration,
Some(escalated_duration),
);
}
Err(err)
}
}
}
Err(err) => {
if let Some(outcome) = sandbox_outcome_from_tool_error(&err) {
otel.sandbox_outcome(
&otel_tn,
otel_ci,
outcome,
initial_duration,
/*escalated_duration*/ None,
);
}
Err(err)
}
Err(err) => Err(err),
}
}

Expand Down Expand Up @@ -509,6 +576,15 @@ impl ToolOrchestrator {
}
}

fn sandbox_outcome_from_tool_error(err: &ToolError) -> Option<&'static str> {
match err {
ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { .. })) => Some("denied"),
ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { .. })) => Some("timed_out"),
ToolError::Codex(CodexErr::Sandbox(SandboxErr::Signal(_))) => Some("signal"),
ToolError::Rejected(_) | ToolError::Codex(_) => None,
}
}

fn build_denial_reason_from_output(_output: &ExecToolCallOutput) -> String {
// Keep approval reason terse and stable for UX/tests, but accept the
// output so we can evolve heuristics later without touching call sites.
Expand Down
69 changes: 69 additions & 0 deletions codex-rs/core/tests/suite/otel.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
use codex_core::config::Constrained;
use codex_features::Feature;
use codex_otel::SessionTelemetry;
use codex_otel::TelemetryAuthMode;
use codex_protocol::ThreadId;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SessionSource;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
Expand All @@ -27,6 +31,7 @@ use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use std::sync::Mutex;
use std::time::Duration;
use tracing::Level;
use tracing_test::traced_test;

Expand Down Expand Up @@ -1145,6 +1150,70 @@ fn tool_decision_assertion<'a>(
}
}

fn sandbox_outcome_assertion<'a>(
call_id: &'a str,
expected_outcome: &'a str,
) -> impl Fn(&[&str]) -> Result<(), String> + 'a {
let call_id = call_id.to_string();
let expected_outcome = expected_outcome.to_string();

move |lines: &[&str]| {
let line = lines
.iter()
.find(|line| {
line.contains("codex.sandbox_outcome")
&& line.contains(&format!("call_id={call_id}"))
})
.ok_or_else(|| format!("missing codex.sandbox_outcome event for {call_id}"))?;

let lower = line.to_lowercase();
if !lower.contains("tool_name=shell_command") {
return Err("missing tool_name for shell_command".to_string());
}
if !lower.contains(&format!("outcome={expected_outcome}")) {
return Err(format!("unexpected sandbox outcome for {call_id}"));
}
if !lower.contains("initial_duration_ms=12") {
return Err("missing initial_duration_ms field".to_string());
}
if !lower.contains("escalated_duration_ms=34") {
return Err("missing escalated_duration_ms field".to_string());
}

Ok(())
}
}

#[test]
#[traced_test]
fn sandbox_outcome_event_records_outcome() {
let telemetry = SessionTelemetry::new(
ThreadId::new(),
"gpt-5.5",
"gpt-5.5",
/*account_id*/ None,
/*account_email*/ None,
Some(TelemetryAuthMode::ApiKey),
"Codex_Desktop".to_string(),
/*log_user_prompts*/ false,
"tty".to_string(),
SessionSource::Cli,
);

telemetry.sandbox_outcome(
"shell_command",
"sandbox-outcome-call",
"escalated",
Duration::from_millis(/*millis*/ 12),
Some(Duration::from_millis(/*millis*/ 34)),
);

logs_assert(sandbox_outcome_assertion(
"sandbox-outcome-call",
"escalated",
));
}

#[tokio::test]
#[traced_test]
async fn handle_shell_command_autoapprove_from_config_records_tool_decision() {
Expand Down
31 changes: 31 additions & 0 deletions codex-rs/otel/src/events/session_telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,37 @@ impl SessionTelemetry {
);
}

pub fn sandbox_outcome(
&self,
tool_name: &str,
call_id: &str,
outcome: &str,
initial_duration: Duration,
escalated_duration: Option<Duration>,
) {
let initial_duration_ms = initial_duration.as_millis().min(i64::MAX as u128) as i64;
let escalated_duration_ms =
escalated_duration.map(|duration| duration.as_millis().min(i64::MAX as u128) as i64);
log_event!(
self,
event.name = "codex.sandbox_outcome",
tool_name = %tool_name,
call_id = %call_id,
outcome = %outcome,
initial_duration_ms = initial_duration_ms,
escalated_duration_ms = escalated_duration_ms,
);
trace_event!(
self,
event.name = "codex.sandbox_outcome",
tool_name = %tool_name,
call_id = %call_id,
outcome = %outcome,
initial_duration_ms = initial_duration_ms,
escalated_duration_ms = escalated_duration_ms,
);
}

#[allow(clippy::too_many_arguments)]
pub async fn log_tool_result_with_tags<F, Fut, E>(
&self,
Expand Down
Loading