Restore otel

This commit is contained in:
jimmyfraiture
2025-09-30 13:28:25 +01:00
parent 9c194dc0f9
commit 5c00e1596a
4 changed files with 64 additions and 23 deletions

View File

@@ -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<String>,
pub(crate) cwd: PathBuf,
pub(crate) apply_patch: Option<ApplyPatchCommandContext>,
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 {

View File

@@ -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<ExecToolCallOutput, ExecError> {
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<StdoutStream>,
sandbox_error: SandboxErr,
) -> Result<ExecToolCallOutput, ExecError> {
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

View File

@@ -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<SandboxDecision, ExecError> {
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<SandboxDecision, ExecError> {
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");

View File

@@ -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,
}
);
}