mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Consolidate approval telemetry context (#36825)
## What changed Add the tool name to `ApprovalCtx` and use the context's call ID and session telemetry when recording approval decisions. This removes redundant `ToolCtx` and telemetry parameters from `resolve_tool_approval`. GitOrigin-RevId: 238a6f708f83993d64606bfaa74df344b0b46178
This commit is contained in:
@@ -9,7 +9,6 @@ use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::tools::flat_tool_name;
|
||||
use crate::tools::sandboxing::ApprovalCtx;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
@@ -193,9 +192,7 @@ pub(super) async fn resolve_tool_approval<Rq, Out, T>(
|
||||
req: &Rq,
|
||||
permission_request_run_id: &str,
|
||||
ctx: ApprovalCtx<'_>,
|
||||
tool_ctx: &ToolCtx,
|
||||
reviewer: ApprovalReviewer,
|
||||
otel: &codex_otel::SessionTelemetry,
|
||||
) -> Result<ReviewDecision, ToolError>
|
||||
where
|
||||
T: ToolRuntime<Rq, Out>,
|
||||
@@ -214,7 +211,7 @@ where
|
||||
decision: ReviewDecision::Approved,
|
||||
source: ApprovalResolutionSource::Hook,
|
||||
};
|
||||
record_resolution(otel, tool_ctx, &resolution);
|
||||
record_resolution(&ctx, &resolution);
|
||||
return resolution.into_tool_result();
|
||||
}
|
||||
Some(PermissionRequestDecision::Deny { message }) => {
|
||||
@@ -222,7 +219,7 @@ where
|
||||
decision: ReviewDecision::denied(message),
|
||||
source: ApprovalResolutionSource::Hook,
|
||||
};
|
||||
record_resolution(otel, tool_ctx, &resolution);
|
||||
record_resolution(&ctx, &resolution);
|
||||
return resolution.into_tool_result();
|
||||
}
|
||||
None => {}
|
||||
@@ -245,7 +242,7 @@ where
|
||||
),
|
||||
source: ApprovalResolutionSource::Guardian,
|
||||
};
|
||||
record_resolution(otel, tool_ctx, &resolution);
|
||||
record_resolution(&ctx, &resolution);
|
||||
return resolution.into_tool_result();
|
||||
}
|
||||
};
|
||||
@@ -268,24 +265,20 @@ where
|
||||
decision: resolution,
|
||||
source,
|
||||
};
|
||||
record_resolution(otel, tool_ctx, &resolution);
|
||||
record_resolution(&ctx, &resolution);
|
||||
resolution.into_tool_result()
|
||||
}
|
||||
|
||||
fn record_resolution(
|
||||
otel: &codex_otel::SessionTelemetry,
|
||||
tool_ctx: &ToolCtx,
|
||||
resolution: &ApprovalResolution,
|
||||
) {
|
||||
fn record_resolution(ctx: &ApprovalCtx<'_>, resolution: &ApprovalResolution) {
|
||||
let source = match resolution.source {
|
||||
ApprovalResolutionSource::Hook => ToolDecisionSource::Config,
|
||||
ApprovalResolutionSource::Guardian => ToolDecisionSource::AutomatedReviewer,
|
||||
ApprovalResolutionSource::User => ToolDecisionSource::User,
|
||||
};
|
||||
let tool_name = flat_tool_name(&tool_ctx.tool_name);
|
||||
otel.tool_decision(
|
||||
let tool_name = flat_tool_name(ctx.tool_name);
|
||||
ctx.turn.session_telemetry.tool_decision(
|
||||
tool_name.as_ref(),
|
||||
&tool_ctx.call_id,
|
||||
ctx.call_id,
|
||||
&resolution.decision,
|
||||
source,
|
||||
);
|
||||
|
||||
@@ -171,6 +171,7 @@ impl ToolOrchestrator {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
tool_name: &tool_ctx.tool_name,
|
||||
retry_reason: None,
|
||||
network_approval_context: None,
|
||||
};
|
||||
@@ -179,9 +180,7 @@ impl ToolOrchestrator {
|
||||
req,
|
||||
tool_ctx.call_id.as_str(),
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
ApprovalReviewer::Guardian,
|
||||
&otel,
|
||||
)
|
||||
.await?;
|
||||
already_approved = true;
|
||||
@@ -202,6 +201,7 @@ impl ToolOrchestrator {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
tool_name: &tool_ctx.tool_name,
|
||||
retry_reason: reason.clone(),
|
||||
network_approval_context: None,
|
||||
};
|
||||
@@ -210,13 +210,11 @@ impl ToolOrchestrator {
|
||||
req,
|
||||
tool_ctx.call_id.as_str(),
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
if strict_auto_review {
|
||||
ApprovalReviewer::Guardian
|
||||
} else {
|
||||
ApprovalReviewer::for_turn(turn_ctx)
|
||||
},
|
||||
&otel,
|
||||
)
|
||||
.await?;
|
||||
already_approved = true;
|
||||
@@ -397,6 +395,7 @@ impl ToolOrchestrator {
|
||||
session: &tool_ctx.session,
|
||||
turn: &tool_ctx.turn,
|
||||
call_id: &tool_ctx.call_id,
|
||||
tool_name: &tool_ctx.tool_name,
|
||||
retry_reason: Some(retry_reason),
|
||||
network_approval_context: network_approval_context.clone(),
|
||||
};
|
||||
@@ -407,13 +406,11 @@ impl ToolOrchestrator {
|
||||
req,
|
||||
&permission_request_run_id,
|
||||
approval_ctx,
|
||||
tool_ctx,
|
||||
if strict_auto_review {
|
||||
ApprovalReviewer::Guardian
|
||||
} else {
|
||||
ApprovalReviewer::for_turn(turn_ctx)
|
||||
},
|
||||
&otel,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
@@ -121,6 +121,7 @@ pub(crate) struct ApprovalCtx<'a> {
|
||||
pub session: &'a Arc<Session>,
|
||||
pub turn: &'a Arc<TurnContext>,
|
||||
pub call_id: &'a str,
|
||||
pub tool_name: &'a ToolName,
|
||||
pub retry_reason: Option<String>,
|
||||
pub network_approval_context: Option<NetworkApprovalContext>,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user