openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-02 19:43:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

remove approval metadata from sandbox policy branch

Browse Source
This commit is contained in:
Roy Han
2026-03-23 11:08:00 -07:00
parent dcbeb4ade4
commit 5a0d6230c8
12 changed files with 27 additions and 447 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/schema/json/ClientRequest.json
-38
View File
@@ -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",
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
-38
View File
@@ -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",
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json
-38
View File
@@ -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",
codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json
-38
View File
@@ -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",
codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json
-38
View File
@@ -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",
codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts
+1 -3
View File
@@ -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, };
codex-rs/app-server-protocol/schema/typescript/ReviewDecisionMetadata.ts
-5
View File
@@ -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";
codex-rs/app-server-protocol/schema/typescript/index.ts
-1
View File
@@ -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";
codex-rs/core/src/codex.rs
+23 -108
View File
@@ -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,
codex-rs/core/src/codex_tests.rs
+3 -80
View File
@@ -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)
)
));
}
codex-rs/core/src/state/turn.rs
-38
View File
@@ -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<String, oneshot::Sender<ReviewDecision>>,
pending_approval_call_ids: HashMap<String, String>,
approval_outcomes_by_call_id: HashMap<String, ReviewDecisionMetadata>,
pending_request_permissions: HashMap<String, oneshot::Sender<RequestPermissionsResponse>>,
pending_user_input: HashMap<String, oneshot::Sender<RequestUserInputResponse>>,
pending_elicitations: HashMap<(String, RequestId), oneshot::Sender<ElicitationResponse>>,
@@ -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<String> {
self.pending_approval_call_ids.insert(approval_key, call_id)
}
pub(crate) fn remove_pending_approval_call_id(&mut self, approval_key: &str) -> Option<String> {
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<ReviewDecisionMetadata> {
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();
codex-rs/protocol/src/models.rs
-22
View File
@@ -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<UserMessageType>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub sandbox_policy: Option<SandboxPolicyMetadata>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub is_tool_call_escalated: Option<bool>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub review_decision: Option<ReviewDecisionMetadata>,
}
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]