From 144fcbe295fb0455b8cf0b927d9038adae1450bb Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 13 Apr 2026 17:04:04 -0700 Subject: [PATCH] Drop PermissionRequestApprovalAttempt Remove the approval-attempt enum and let callers provide the final permission-request hook run id suffix directly. This keeps retry hook runs unique without carrying an extra cross-crate type.\n\nCo-authored-by: Codex --- codex-rs/core/src/hook_runtime.rs | 6 ++-- codex-rs/core/src/tools/network_approval.rs | 2 -- codex-rs/core/src/tools/orchestrator.rs | 10 +++--- .../tools/runtimes/shell/unix_escalation.rs | 2 -- .../hooks/src/events/permission_request.rs | 34 ++++--------------- codex-rs/hooks/src/lib.rs | 1 - 6 files changed, 13 insertions(+), 42 deletions(-) diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 5e81c939df..80f0d68d2a 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -1,7 +1,6 @@ use std::future::Future; use std::sync::Arc; -use codex_hooks::PermissionRequestApprovalAttempt; use codex_hooks::PermissionRequestDecision; use codex_hooks::PermissionRequestOutcome; use codex_hooks::PermissionRequestRequest; @@ -156,9 +155,8 @@ pub(crate) async fn run_pre_tool_use_hooks( pub(crate) async fn run_permission_request_hooks( sess: &Arc, turn_context: &Arc, - request_id: &str, + run_id_suffix: &str, payload: PermissionRequestPayload, - approval_attempt: PermissionRequestApprovalAttempt, ) -> Option { let request = PermissionRequestRequest { session_id: sess.conversation_id, @@ -168,7 +166,7 @@ pub(crate) async fn run_permission_request_hooks( model: turn_context.model_info.slug.clone(), permission_mode: hook_permission_mode(turn_context), tool_name: payload.tool_name, - run_id_suffix: format!("{request_id}:{}", approval_attempt.as_wire_value()), + run_id_suffix: run_id_suffix.to_string(), command: payload.command, description: payload.description, }; diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 110f1a426a..bb585cbc95 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -9,7 +9,6 @@ use crate::hook_runtime::run_permission_request_hooks; use crate::network_policy_decision::denied_network_policy_message; use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::ToolError; -use codex_hooks::PermissionRequestApprovalAttempt; use codex_hooks::PermissionRequestDecision; use codex_network_proxy::BlockedRequest; use codex_network_proxy::BlockedRequestObserver; @@ -391,7 +390,6 @@ impl NetworkApprovalService { command, description: Some(format!("network-access {target}")), }, - PermissionRequestApprovalAttempt::Initial, ) .await { diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 70ee9b3790..7d3be77719 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -25,7 +25,6 @@ use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::default_exec_approval_requirement; -use codex_hooks::PermissionRequestApprovalAttempt; use codex_hooks::PermissionRequestDecision; use codex_otel::SessionTelemetry; use codex_otel::ToolDecisionSource; @@ -157,7 +156,7 @@ impl ToolOrchestrator { let decision = Self::request_approval( tool, req, - PermissionRequestApprovalAttempt::Initial, + tool_ctx.call_id.as_str(), approval_ctx, turn_ctx, ApprovalTelemetry { @@ -302,7 +301,7 @@ impl ToolOrchestrator { let decision = Self::request_approval( tool, req, - PermissionRequestApprovalAttempt::Retry, + &format!("{}:retry", tool_ctx.call_id), approval_ctx, turn_ctx, ApprovalTelemetry { @@ -361,7 +360,7 @@ impl ToolOrchestrator { async fn request_approval( tool: &mut T, req: &Rq, - approval_attempt: PermissionRequestApprovalAttempt, + permission_request_run_id: &str, approval_ctx: ApprovalCtx<'_>, turn_ctx: &crate::codex::TurnContext, telemetry: ApprovalTelemetry<'_>, @@ -373,9 +372,8 @@ impl ToolOrchestrator { match run_permission_request_hooks( approval_ctx.session, approval_ctx.turn, - approval_ctx.call_id, + permission_request_run_id, permission_request, - approval_attempt, ) .await { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 1ee17b4d07..836cd2a114 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -24,7 +24,6 @@ use codex_execpolicy::MatchOptions; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_features::Feature; -use codex_hooks::PermissionRequestApprovalAttempt; use codex_hooks::PermissionRequestDecision; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::error::CodexErr; @@ -411,7 +410,6 @@ impl CoreShellActionProvider { &turn, &effective_approval_id, permission_request, - PermissionRequestApprovalAttempt::Initial, ) .await { diff --git a/codex-rs/hooks/src/events/permission_request.rs b/codex-rs/hooks/src/events/permission_request.rs index 018b0dcd9b..c6b58b86f1 100644 --- a/codex-rs/hooks/src/events/permission_request.rs +++ b/codex-rs/hooks/src/events/permission_request.rs @@ -15,17 +15,6 @@ //! allow wins, otherwise there is no hook verdict. use std::path::PathBuf; -use codex_protocol::ThreadId; -use codex_protocol::protocol::HookCompletedEvent; -use codex_protocol::protocol::HookEventName; -use codex_protocol::protocol::HookOutputEntry; -use codex_protocol::protocol::HookOutputEntryKind; -use codex_protocol::protocol::HookRunStatus; -use codex_protocol::protocol::HookRunSummary; -use schemars::JsonSchema; -use serde::Deserialize; -use serde::Serialize; - use super::common; use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; @@ -34,6 +23,13 @@ use crate::engine::dispatcher; use crate::engine::output_parser; use crate::schema::PermissionRequestCommandInput; use crate::schema::PermissionRequestToolInput; +use codex_protocol::ThreadId; +use codex_protocol::protocol::HookCompletedEvent; +use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookOutputEntry; +use codex_protocol::protocol::HookOutputEntryKind; +use codex_protocol::protocol::HookRunStatus; +use codex_protocol::protocol::HookRunSummary; #[derive(Debug, Clone)] pub struct PermissionRequestRequest { @@ -49,22 +45,6 @@ pub struct PermissionRequestRequest { pub description: Option, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub enum PermissionRequestApprovalAttempt { - Initial, - Retry, -} - -impl PermissionRequestApprovalAttempt { - pub const fn as_wire_value(self) -> &'static str { - match self { - Self::Initial => "initial", - Self::Retry => "retry", - } - } -} - #[derive(Debug, Clone, PartialEq, Eq)] pub enum PermissionRequestDecision { Allow, diff --git a/codex-rs/hooks/src/lib.rs b/codex-rs/hooks/src/lib.rs index e21d09a332..c8358c678c 100644 --- a/codex-rs/hooks/src/lib.rs +++ b/codex-rs/hooks/src/lib.rs @@ -5,7 +5,6 @@ mod registry; mod schema; mod types; -pub use events::permission_request::PermissionRequestApprovalAttempt; pub use events::permission_request::PermissionRequestDecision; pub use events::permission_request::PermissionRequestOutcome; pub use events::permission_request::PermissionRequestRequest;