diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 1572143f07..d249b12047 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -2024,22 +2024,6 @@ }, "ResponseItemMetadata": { "properties": { - "is_tool_call_escalated": { - "type": [ - "boolean", - "null" - ] - }, - "review_decision": { - "anyOf": [ - { - "$ref": "#/definitions/ReviewDecisionMetadata" - }, - { - "type": "null" - } - ] - }, "sandbox_policy": { "anyOf": [ { @@ -2049,16 +2033,6 @@ "type": "null" } ] - }, - "user_message_type": { - "anyOf": [ - { - "$ref": "#/definitions/UserMessageType" - }, - { - "type": "null" - } - ] } }, "type": "object" @@ -2164,18 +2138,6 @@ } ] }, - "ReviewDecisionMetadata": { - "enum": [ - "approved", - "denied", - "abort", - "approved_for_session", - "approved_with_amendment", - "approved_with_network_policy_allow", - "denied_with_network_policy_deny" - ], - "type": "string" - }, "ReviewDelivery": { "enum": [ "inline", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 3fe047a577..f4926b28e3 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -10747,22 +10747,6 @@ }, "ResponseItemMetadata": { "properties": { - "is_tool_call_escalated": { - "type": [ - "boolean", - "null" - ] - }, - "review_decision": { - "anyOf": [ - { - "$ref": "#/definitions/v2/ReviewDecisionMetadata" - }, - { - "type": "null" - } - ] - }, "sandbox_policy": { "anyOf": [ { @@ -10772,16 +10756,6 @@ "type": "null" } ] - }, - "user_message_type": { - "anyOf": [ - { - "$ref": "#/definitions/v2/UserMessageType" - }, - { - "type": "null" - } - ] } }, "type": "object" @@ -10887,18 +10861,6 @@ } ] }, - "ReviewDecisionMetadata": { - "enum": [ - "approved", - "denied", - "abort", - "approved_for_session", - "approved_with_amendment", - "approved_with_network_policy_allow", - "denied_with_network_policy_deny" - ], - "type": "string" - }, "ReviewDelivery": { "enum": [ "inline", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 654b1e6049..53fd720b68 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -7515,22 +7515,6 @@ }, "ResponseItemMetadata": { "properties": { - "is_tool_call_escalated": { - "type": [ - "boolean", - "null" - ] - }, - "review_decision": { - "anyOf": [ - { - "$ref": "#/definitions/ReviewDecisionMetadata" - }, - { - "type": "null" - } - ] - }, "sandbox_policy": { "anyOf": [ { @@ -7540,16 +7524,6 @@ "type": "null" } ] - }, - "user_message_type": { - "anyOf": [ - { - "$ref": "#/definitions/UserMessageType" - }, - { - "type": "null" - } - ] } }, "type": "object" @@ -7655,18 +7629,6 @@ } ] }, - "ReviewDecisionMetadata": { - "enum": [ - "approved", - "denied", - "abort", - "approved_for_session", - "approved_with_amendment", - "approved_with_network_policy_allow", - "denied_with_network_policy_deny" - ], - "type": "string" - }, "ReviewDelivery": { "enum": [ "inline", diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index 21abd9b0c6..fe2fda776b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -872,22 +872,6 @@ }, "ResponseItemMetadata": { "properties": { - "is_tool_call_escalated": { - "type": [ - "boolean", - "null" - ] - }, - "review_decision": { - "anyOf": [ - { - "$ref": "#/definitions/ReviewDecisionMetadata" - }, - { - "type": "null" - } - ] - }, "sandbox_policy": { "anyOf": [ { @@ -897,16 +881,6 @@ "type": "null" } ] - }, - "user_message_type": { - "anyOf": [ - { - "$ref": "#/definitions/UserMessageType" - }, - { - "type": "null" - } - ] } }, "type": "object" @@ -1012,18 +986,6 @@ } ] }, - "ReviewDecisionMetadata": { - "enum": [ - "approved", - "denied", - "abort", - "approved_for_session", - "approved_with_amendment", - "approved_with_network_policy_allow", - "denied_with_network_policy_deny" - ], - "type": "string" - }, "SandboxPolicyMetadata": { "enum": [ "read_only", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index 9ea4cf95a7..7c54c7b652 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -938,22 +938,6 @@ }, "ResponseItemMetadata": { "properties": { - "is_tool_call_escalated": { - "type": [ - "boolean", - "null" - ] - }, - "review_decision": { - "anyOf": [ - { - "$ref": "#/definitions/ReviewDecisionMetadata" - }, - { - "type": "null" - } - ] - }, "sandbox_policy": { "anyOf": [ { @@ -963,16 +947,6 @@ "type": "null" } ] - }, - "user_message_type": { - "anyOf": [ - { - "$ref": "#/definitions/UserMessageType" - }, - { - "type": "null" - } - ] } }, "type": "object" @@ -1078,18 +1052,6 @@ } ] }, - "ReviewDecisionMetadata": { - "enum": [ - "approved", - "denied", - "abort", - "approved_for_session", - "approved_with_amendment", - "approved_with_network_policy_allow", - "denied_with_network_policy_deny" - ], - "type": "string" - }, "SandboxMode": { "enum": [ "read-only", diff --git a/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts index 4f3ffa0405..9a44e10720 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts @@ -1,8 +1,6 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { ReviewDecisionMetadata } from "./ReviewDecisionMetadata"; import type { SandboxPolicyMetadata } from "./SandboxPolicyMetadata"; -import type { UserMessageType } from "./UserMessageType"; -export type ResponseItemMetadata = { user_message_type?: UserMessageType, sandbox_policy?: SandboxPolicyMetadata, is_tool_call_escalated?: boolean, review_decision?: ReviewDecisionMetadata, }; +export type ResponseItemMetadata = { sandbox_policy?: SandboxPolicyMetadata, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/ReviewDecisionMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/ReviewDecisionMetadata.ts deleted file mode 100644 index 7a5493e140..0000000000 --- a/codex-rs/app-server-protocol/schema/typescript/ReviewDecisionMetadata.ts +++ /dev/null @@ -1,5 +0,0 @@ -// GENERATED CODE! DO NOT MODIFY BY HAND! - -// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. - -export type ReviewDecisionMetadata = "approved" | "denied" | "abort" | "approved_for_session" | "approved_with_amendment" | "approved_with_network_policy_allow" | "denied_with_network_policy_deny"; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index a963406a5c..4ceca79912 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -62,7 +62,6 @@ export type { ResponseItem } from "./ResponseItem"; export type { ResponseItemMessageMetadata } from "./ResponseItemMessageMetadata"; export type { ResponseItemMetadata } from "./ResponseItemMetadata"; export type { ReviewDecision } from "./ReviewDecision"; -export type { ReviewDecisionMetadata } from "./ReviewDecisionMetadata"; export type { SandboxPolicyMetadata } from "./SandboxPolicyMetadata"; export type { ServerNotification } from "./ServerNotification"; export type { ServerRequest } from "./ServerRequest"; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index d7eb067f09..470d3d7636 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -335,7 +335,6 @@ use codex_protocol::models::DeveloperInstructions; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::models::ResponseItemMetadata; -use codex_protocol::models::ReviewDecisionMetadata; use codex_protocol::models::SandboxPolicyMetadata; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::CodexErrorInfo; @@ -1007,28 +1006,6 @@ fn local_time_context() -> (String, String) { } } -fn review_decision_to_metadata(decision: &ReviewDecision) -> ReviewDecisionMetadata { - match decision { - ReviewDecision::Approved => ReviewDecisionMetadata::Approved, - ReviewDecision::ApprovedExecpolicyAmendment { .. } => { - ReviewDecisionMetadata::ApprovedWithAmendment - } - ReviewDecision::ApprovedForSession => ReviewDecisionMetadata::ApprovedForSession, - ReviewDecision::NetworkPolicyAmendment { - network_policy_amendment, - } => match network_policy_amendment.action { - codex_protocol::protocol::NetworkPolicyRuleAction::Allow => { - ReviewDecisionMetadata::ApprovedWithNetworkPolicyAllow - } - codex_protocol::protocol::NetworkPolicyRuleAction::Deny => { - ReviewDecisionMetadata::DeniedWithNetworkPolicyDeny - } - }, - ReviewDecision::Denied => ReviewDecisionMetadata::Denied, - ReviewDecision::Abort => ReviewDecisionMetadata::Abort, - } -} - fn sandbox_policy_to_metadata(policy: &SandboxPolicy) -> SandboxPolicyMetadata { match policy { SandboxPolicy::ReadOnly { .. } => SandboxPolicyMetadata::ReadOnly, @@ -1039,18 +1016,6 @@ fn sandbox_policy_to_metadata(policy: &SandboxPolicy) -> SandboxPolicyMetadata { } } -fn response_item_tool_call_id(item: &ResponseItem) -> Option<&str> { - match item { - ResponseItem::LocalShellCall { - call_id: Some(call_id), - .. - } => Some(call_id), - ResponseItem::FunctionCall { call_id, .. } => Some(call_id), - ResponseItem::CustomToolCall { call_id, .. } => Some(call_id), - _ => None, - } -} - fn stamp_tool_metadata_on_response_item( item: ResponseItem, metadata: ResponseItemMetadata, @@ -1102,6 +1067,24 @@ fn stamp_tool_metadata_on_response_item( other => other, } } + +fn stamp_tool_sandbox_policy_on_response_item( + turn_context: &TurnContext, + response_item: ResponseItem, +) -> ResponseItem { + let metadata = match &response_item { + ResponseItem::LocalShellCall { metadata, .. } + | ResponseItem::FunctionCall { metadata, .. } + | ResponseItem::CustomToolCall { metadata, .. } => metadata.clone().unwrap_or_default(), + _ => return response_item, + }; + + let mut metadata = metadata; + metadata.sandbox_policy = Some(sandbox_policy_to_metadata( + turn_context.sandbox_policy.get(), + )); + stamp_tool_metadata_on_response_item(response_item, metadata) +} #[derive(Clone)] pub(crate) struct SessionConfiguration { /// Provider identifier ("openai", "openrouter", ...). @@ -2968,10 +2951,6 @@ impl Session { match active.as_mut() { Some(at) => { let mut ts = at.turn_state.lock().await; - ts.insert_pending_approval_call_id( - effective_approval_id.clone(), - call_id.clone(), - ); ts.insert_pending_approval(effective_approval_id.clone(), tx_approve) } None => None, @@ -3037,7 +3016,6 @@ impl Session { match active.as_mut() { Some(at) => { let mut ts = at.turn_state.lock().await; - ts.insert_pending_approval_call_id(approval_id.clone(), call_id.clone()); ts.insert_pending_approval(approval_id.clone(), tx_approve) } None => None, @@ -3323,20 +3301,7 @@ impl Session { } } - pub async fn record_approval_outcome(&self, approval_id: &str, decision: &ReviewDecision) { - let mut active = self.active_turn.lock().await; - let Some(at) = active.as_mut() else { - return; - }; - let mut ts = at.turn_state.lock().await; - let call_id = ts - .remove_pending_approval_call_id(approval_id) - .unwrap_or_else(|| approval_id.to_string()); - ts.record_approval_outcome(call_id, review_decision_to_metadata(decision)); - } - pub async fn notify_approval(&self, approval_id: &str, decision: ReviewDecision) { - self.record_approval_outcome(approval_id, &decision).await; let entry = { let mut active = self.active_turn.lock().await; match active.as_mut() { @@ -3357,56 +3322,6 @@ impl Session { } } - async fn stamp_tool_approval_metadata( - &self, - turn_context: &TurnContext, - response_item: ResponseItem, - ) -> ResponseItem { - if !self.enabled(Feature::ItemMetadata) { - return response_item; - } - let Some(call_id) = response_item_tool_call_id(&response_item) else { - return response_item; - }; - - let (outcome, has_pending_approval) = { - let active = self.active_turn.lock().await; - let Some(at) = active.as_ref() else { - return response_item; - }; - let ts = at.turn_state.lock().await; - let outcome = ts.approval_outcome_for_call_id(call_id); - let has_pending_approval = ts.has_pending_approval_for_call_id(call_id); - (outcome, has_pending_approval) - }; - - let mut metadata = match &response_item { - ResponseItem::LocalShellCall { metadata, .. } - | ResponseItem::FunctionCall { metadata, .. } - | ResponseItem::CustomToolCall { metadata, .. } => metadata.clone().unwrap_or_default(), - _ => return response_item, - }; - metadata.sandbox_policy = Some(sandbox_policy_to_metadata( - turn_context.sandbox_policy.get(), - )); - - match outcome { - Some(review_decision) => { - metadata.is_tool_call_escalated = Some(true); - metadata.review_decision = Some(review_decision); - } - None if !has_pending_approval => { - metadata.is_tool_call_escalated = Some(false); - metadata.review_decision = None; - } - None => { - return response_item; - } - } - - stamp_tool_metadata_on_response_item(response_item, metadata) - } - pub async fn resolve_elicitation( &self, server_name: String, @@ -3959,9 +3874,11 @@ impl Session { turn_context: &TurnContext, response_item: ResponseItem, ) { - let response_item = self - .stamp_tool_approval_metadata(turn_context, response_item) - .await; + let response_item = if self.enabled(Feature::ItemMetadata) { + stamp_tool_sandbox_policy_on_response_item(turn_context, response_item) + } else { + response_item + }; // Add to conversation history and persist response item to rollout. self.record_conversation_items(turn_context, std::slice::from_ref(&response_item)) @@ -4896,8 +4813,6 @@ mod handlers { } match decision { ReviewDecision::Abort => { - sess.record_approval_outcome(&approval_id, &ReviewDecision::Abort) - .await; sess.interrupt_task().await; } other => sess.notify_approval(&approval_id, other).await, diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 0c3ffbf7b9..83e1abe3a1 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -4414,85 +4414,8 @@ async fn task_finish_emits_prompt_queued_metadata_for_injected_user_input_when_f )); } -#[test] -fn review_decision_metadata_mapping_is_stable() { - assert_eq!( - review_decision_to_metadata(&ReviewDecision::Approved), - codex_protocol::models::ReviewDecisionMetadata::Approved - ); - assert_eq!( - review_decision_to_metadata(&ReviewDecision::Denied), - codex_protocol::models::ReviewDecisionMetadata::Denied - ); - assert_eq!( - review_decision_to_metadata(&ReviewDecision::Abort), - codex_protocol::models::ReviewDecisionMetadata::Abort - ); - assert_eq!( - review_decision_to_metadata(&ReviewDecision::ApprovedForSession), - codex_protocol::models::ReviewDecisionMetadata::ApprovedForSession - ); -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn tool_call_metadata_stamps_escalated_review_decision_when_feature_enabled() { - let (mut sess, tc, rx) = make_session_and_context_with_rx().await; - Arc::get_mut(&mut sess) - .expect("session should be uniquely owned in this test") - .features - .enable(crate::features::Feature::ItemMetadata) - .expect("feature flag should be enabled for this test"); - - sess.spawn_task( - Arc::clone(&tc), - vec![UserInput::Text { - text: "start".to_string(), - text_elements: Vec::new(), - }], - NeverEndingTask { - kind: TaskKind::Regular, - listen_to_cancellation_token: false, - }, - ) - .await; - while rx.try_recv().is_ok() {} - - sess.record_approval_outcome("call-1", &ReviewDecision::Denied) - .await; - sess.record_response_item_and_emit_turn_item( - tc.as_ref(), - ResponseItem::FunctionCall { - id: None, - name: "shell".to_string(), - namespace: None, - arguments: "{}".to_string(), - call_id: "call-1".to_string(), - metadata: None, - }, - ) - .await; - - let event = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv()) - .await - .expect("expected raw response item event") - .expect("channel open"); - assert!(matches!( - event.msg, - EventMsg::RawResponseItem(ref ev) - if matches!( - &ev.item, - ResponseItem::FunctionCall { - metadata: Some(metadata), - .. - } if metadata.is_tool_call_escalated == Some(true) - && metadata.review_decision - == Some(codex_protocol::models::ReviewDecisionMetadata::Denied) - ) - )); -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn tool_call_metadata_stamps_non_escalated_false_when_feature_enabled() { +async fn tool_call_metadata_stamps_sandbox_policy_when_feature_enabled() { let (mut sess, tc, rx) = make_session_and_context_with_rx().await; Arc::get_mut(&mut sess) .expect("session should be uniquely owned in this test") @@ -4539,8 +4462,8 @@ async fn tool_call_metadata_stamps_non_escalated_false_when_feature_enabled() { ResponseItem::FunctionCall { metadata: Some(metadata), .. - } if metadata.is_tool_call_escalated == Some(false) - && metadata.review_decision.is_none() + } if metadata.sandbox_policy + == Some(codex_protocol::models::SandboxPolicyMetadata::ReadOnly) ) )); } diff --git a/codex-rs/core/src/state/turn.rs b/codex-rs/core/src/state/turn.rs index d026db0ca0..a6ae1ba8ca 100644 --- a/codex-rs/core/src/state/turn.rs +++ b/codex-rs/core/src/state/turn.rs @@ -10,7 +10,6 @@ use tokio_util::task::AbortOnDropHandle; use codex_protocol::dynamic_tools::DynamicToolResponse; use codex_protocol::models::ResponseInputItem; -use codex_protocol::models::ReviewDecisionMetadata; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationResponse; @@ -77,8 +76,6 @@ impl ActiveTurn { #[derive(Default)] pub(crate) struct TurnState { pending_approvals: HashMap>, - pending_approval_call_ids: HashMap, - approval_outcomes_by_call_id: HashMap, pending_request_permissions: HashMap>, pending_user_input: HashMap>, pending_elicitations: HashMap<(String, RequestId), oneshot::Sender>, @@ -98,39 +95,6 @@ impl TurnState { self.pending_approvals.insert(key, tx) } - pub(crate) fn insert_pending_approval_call_id( - &mut self, - approval_key: String, - call_id: String, - ) -> Option { - self.pending_approval_call_ids.insert(approval_key, call_id) - } - - pub(crate) fn remove_pending_approval_call_id(&mut self, approval_key: &str) -> Option { - self.pending_approval_call_ids.remove(approval_key) - } - - pub(crate) fn record_approval_outcome( - &mut self, - call_id: String, - decision: ReviewDecisionMetadata, - ) { - self.approval_outcomes_by_call_id.insert(call_id, decision); - } - - pub(crate) fn approval_outcome_for_call_id( - &self, - call_id: &str, - ) -> Option { - self.approval_outcomes_by_call_id.get(call_id).cloned() - } - - pub(crate) fn has_pending_approval_for_call_id(&self, call_id: &str) -> bool { - self.pending_approval_call_ids - .values() - .any(|pending_call_id| pending_call_id == call_id) - } - pub(crate) fn remove_pending_approval( &mut self, key: &str, @@ -140,8 +104,6 @@ impl TurnState { pub(crate) fn clear_pending(&mut self) { self.pending_approvals.clear(); - self.pending_approval_call_ids.clear(); - self.approval_outcomes_by_call_id.clear(); self.pending_request_permissions.clear(); self.pending_user_input.clear(); self.pending_elicitations.clear(); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index f8251be209..6257a01d78 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -291,19 +291,6 @@ impl ResponseItemMessageMetadata { } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "snake_case")] -#[ts(rename_all = "snake_case")] -pub enum ReviewDecisionMetadata { - Approved, - Denied, - Abort, - ApprovedForSession, - ApprovedWithAmendment, - ApprovedWithNetworkPolicyAllow, - DeniedWithNetworkPolicyDeny, -} - #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] #[ts(rename_all = "snake_case")] @@ -315,18 +302,9 @@ pub enum SandboxPolicyMetadata { #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] pub struct ResponseItemMetadata { - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub user_message_type: Option, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub sandbox_policy: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub is_tool_call_escalated: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub review_decision: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]