agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Add skill approval event/response (#12633)" (#12811)

Browse Source 
This reverts commit https://github.com/openai/codex/pull/12633. We no
longer need this PR, because we favor sending normal exec command
approval server request with `additional_permissions` of skill
permissions instead
This commit is contained in:
Celia Chen
2026-02-25 17:02:42 -08:00
committed by GitHub
Unverified
parent 4fedef88e0
commit 4f45668106
32 changed files with 4 additions and 595 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
-82
View File
@@ -94,7 +94,6 @@ use codex_protocol::protocol::TurnContextNetworkItem;
use codex_protocol::protocol::TurnStartedEvent;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::skill_approval::SkillApprovalResponse;
use codex_rmcp_client::ElicitationResponse;
use codex_rmcp_client::OAuthCredentialsStoreMode;
use codex_utils_stream_parser::AssistantTextChunk;
@@ -2723,40 +2722,6 @@ impl Session {
rx_response.await.ok()
}
pub async fn request_skill_approval(
&self,
turn_context: &TurnContext,
item_id: String,
skill_name: String,
) -> Option<SkillApprovalResponse> {
let (tx_response, rx_response) = oneshot::channel();
let prev_entry = {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
ts.insert_pending_skill_approval(item_id.clone(), tx_response)
}
None => None,
}
};
if prev_entry.is_some() {
warn!("Overwriting existing pending skill approval for item_id: {item_id}");
}
self.send_event(
turn_context,
EventMsg::SkillRequestApproval(
codex_protocol::skill_approval::SkillRequestApprovalEvent {
item_id,
skill_name,
},
),
)
.await;
rx_response.await.ok()
}
pub async fn notify_user_input_response(
&self,
sub_id: &str,
@@ -2803,31 +2768,6 @@ impl Session {
}
}
pub async fn notify_skill_approval_response(
&self,
item_id: &str,
response: SkillApprovalResponse,
) {
let entry = {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
ts.remove_pending_skill_approval(item_id)
}
None => None,
}
};
match entry {
Some(tx_response) => {
tx_response.send(response).ok();
}
None => {
warn!("No pending skill approval found for item_id: {item_id}");
}
}
}
pub async fn notify_approval(&self, approval_id: &str, decision: ReviewDecision) {
let entry = {
let mut active = self.active_turn.lock().await;
@@ -3731,9 +3671,6 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
Op::DynamicToolResponse { id, response } => {
handlers::dynamic_tool_response(&sess, id, response).await;
}
Op::SkillApproval { id, response } => {
handlers::skill_approval_response(&sess, id, response).await;
}
Op::AddToHistory { text } => {
handlers::add_to_history(&sess, &config, text).await;
}
@@ -3857,7 +3794,6 @@ mod handlers {
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::WarningEvent;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::skill_approval::SkillApprovalResponse;
use crate::context_manager::is_user_turn_boundary;
use codex_protocol::config_types::CollaborationMode;
@@ -4098,14 +4034,6 @@ mod handlers {
sess.notify_dynamic_tool_response(&id, response).await;
}
pub async fn skill_approval_response(
sess: &Arc<Session>,
id: String,
response: SkillApprovalResponse,
) {
sess.notify_skill_approval_response(&id, response).await;
}
pub async fn add_to_history(sess: &Arc<Session>, config: &Arc<Config>, text: String) {
let id = sess.conversation_id;
let config = Arc::clone(config);
@@ -4819,15 +4747,6 @@ pub(crate) async fn run_turn(
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, event).await;
if turn_context.config.features.enabled(Feature::SkillApproval) {
let _ = sess
.request_skill_approval(
turn_context.as_ref(),
turn_context.sub_id.clone(),
"test-skill".to_string(),
)
.await;
}
// TODO(ccunningham): Pre-turn compaction runs before context updates and the
// new user message are recorded. Estimate pending incoming items (context
// diffs/full reinjection + user input) and trigger compaction preemptively
@@ -5856,7 +5775,6 @@ fn realtime_text_for_event(msg: &EventMsg) -> Option<String> {
| EventMsg::RequestUserInput(_)
| EventMsg::DynamicToolCallRequest(_)
| EventMsg::DynamicToolCallResponse(_)
| EventMsg::SkillRequestApproval(_)
| EventMsg::ElicitationRequest(_)
| EventMsg::ApplyPatchApprovalRequest(_)
| EventMsg::DeprecationNotice(_)
codex-rs/core/src/features.rs
-8
View File
@@ -133,8 +133,6 @@ pub enum Feature {
SkillMcpDependencyInstall,
/// Prompt for missing skill env var dependencies.
SkillEnvVarDependencyPrompt,
/// Emit skill approval test prompts/events.
SkillApproval,
/// Steer feature flag - when enabled, Enter submits immediately instead of queuing.
/// Kept for config backward compatibility; behavior is always steer-enabled.
Steer,
@@ -630,12 +628,6 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::SkillApproval,
key: "skill_approval",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::Steer,
key: "steer",
codex-rs/core/src/rollout/policy.rs
-1
View File
@@ -141,7 +141,6 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option<EventPersistenceMode> {
| EventMsg::TerminalInteraction(_)
| EventMsg::ExecCommandOutputDelta(_)
| EventMsg::ExecApprovalRequest(_)
| EventMsg::SkillRequestApproval(_)
| EventMsg::RequestUserInput(_)
| EventMsg::ElicitationRequest(_)
| EventMsg::ApplyPatchApprovalRequest(_)
codex-rs/core/src/state/turn.rs
-18
View File
@@ -11,7 +11,6 @@ use tokio_util::task::AbortOnDropHandle;
use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::skill_approval::SkillApprovalResponse;
use tokio::sync::oneshot;
use crate::codex::TurnContext;
@@ -72,7 +71,6 @@ impl ActiveTurn {
pub(crate) struct TurnState {
pending_approvals: HashMap<String, oneshot::Sender<ReviewDecision>>,
pending_user_input: HashMap<String, oneshot::Sender<RequestUserInputResponse>>,
pending_skill_approvals: HashMap<String, oneshot::Sender<SkillApprovalResponse>>,
pending_dynamic_tools: HashMap<String, oneshot::Sender<DynamicToolResponse>>,
pending_input: Vec<ResponseInputItem>,
}
@@ -96,7 +94,6 @@ impl TurnState {
pub(crate) fn clear_pending(&mut self) {
self.pending_approvals.clear();
self.pending_user_input.clear();
self.pending_skill_approvals.clear();
self.pending_dynamic_tools.clear();
self.pending_input.clear();
}
@@ -124,21 +121,6 @@ impl TurnState {
self.pending_dynamic_tools.insert(key, tx)
}
pub(crate) fn insert_pending_skill_approval(
&mut self,
key: String,
tx: oneshot::Sender<SkillApprovalResponse>,
) -> Option<oneshot::Sender<SkillApprovalResponse>> {
self.pending_skill_approvals.insert(key, tx)
}
pub(crate) fn remove_pending_skill_approval(
&mut self,
key: &str,
) -> Option<oneshot::Sender<SkillApprovalResponse>> {
self.pending_skill_approvals.remove(key)
}
pub(crate) fn remove_pending_dynamic_tool(
&mut self,
key: &str,