openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-09 20:50:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

app-server: type network approval parent item ids

Browse Source 
Thread a typed parent tool item id through guardian assessment events and legacy app-server guardian review notifications for network approvals.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Charles Cunningham
2026-03-22 14:59:50 -07:00
co-authored by Codex
parent d66e3dfa19
commit 33077d50e1
15 changed files with 165 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/schema/json/ServerNotification.json
+14
View File
@@ -1352,6 +1352,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/GuardianApprovalReview"
},
@@ -1377,6 +1384,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/GuardianApprovalReview"
},
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
+14
View File
@@ -8199,6 +8199,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/v2/GuardianApprovalReview"
},
@@ -8226,6 +8233,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/v2/GuardianApprovalReview"
},
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json
+14
View File
@@ -4947,6 +4947,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/GuardianApprovalReview"
},
@@ -4974,6 +4981,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/GuardianApprovalReview"
},
codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json
+7
View File
@@ -60,6 +60,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/GuardianApprovalReview"
},
codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json
+7
View File
@@ -60,6 +60,13 @@
"description": "[UNSTABLE] Temporary notification payload for guardian automatic approval review. This shape is expected to change soon.\n\nTODO(ccunningham): Attach guardian review state to the reviewed tool item's lifecycle instead of sending separate standalone review notifications so the app-server API can persist and replay review state via `thread/read`.",
"properties": {
"action": true,
"parentToolItemId": {
"default": null,
"type": [
"string",
"null"
]
},
"review": {
"$ref": "#/definitions/GuardianApprovalReview"
},
codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewCompletedNotification.ts
+1 -1
View File
@@ -12,4 +12,4 @@ import type { GuardianApprovalReview } from "./GuardianApprovalReview";
* lifecycle instead of sending separate standalone review notifications so the
* app-server API can persist and replay review state via `thread/read`.
*/
export type ItemGuardianApprovalReviewCompletedNotification = { threadId: string, turnId: string, targetItemId: string, review: GuardianApprovalReview, action: JsonValue | null, };
export type ItemGuardianApprovalReviewCompletedNotification = { threadId: string, turnId: string, targetItemId: string, parentToolItemId: string | null, review: GuardianApprovalReview, action: JsonValue | null, };
codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewStartedNotification.ts
+1 -1
View File
@@ -12,4 +12,4 @@ import type { GuardianApprovalReview } from "./GuardianApprovalReview";
* lifecycle instead of sending separate standalone review notifications so the
* app-server API can persist and replay review state via `thread/read`.
*/
export type ItemGuardianApprovalReviewStartedNotification = { threadId: string, turnId: string, targetItemId: string, review: GuardianApprovalReview, action: JsonValue | null, };
export type ItemGuardianApprovalReviewStartedNotification = { threadId: string, turnId: string, targetItemId: string, parentToolItemId: string | null, review: GuardianApprovalReview, action: JsonValue | null, };
codex-rs/app-server-protocol/src/protocol/v2.rs
+4
View File
@@ -4839,6 +4839,8 @@ pub struct ItemGuardianApprovalReviewStartedNotification {
pub thread_id: String,
pub turn_id: String,
pub target_item_id: String,
#[serde(default)]
pub parent_tool_item_id: Option<String>,
pub review: GuardianApprovalReview,
pub action: Option<JsonValue>,
}
@@ -4856,6 +4858,8 @@ pub struct ItemGuardianApprovalReviewCompletedNotification {
pub thread_id: String,
pub turn_id: String,
pub target_item_id: String,
#[serde(default)]
pub parent_tool_item_id: Option<String>,
pub review: GuardianApprovalReview,
pub action: Option<JsonValue>,
}
codex-rs/app-server/README.md
+3 -3
View File
@@ -876,10 +876,10 @@ All items emit shared lifecycle events:
- `item/started` — emits the full `item` when a new unit of work begins so the UI can render it immediately; the `item.id` in this payload matches the `itemId` used by deltas.
- `item/completed` — sends the final `item` once that work itself finishes (for example, after a tool call or message completes); treat this as the authoritative execution/result state.
- `item/autoApprovalReview/started` — [UNSTABLE] temporary guardian notification carrying `{threadId, turnId, targetItemId, review, action?}` when guardian approval review begins. This shape is expected to change soon.
- `item/autoApprovalReview/completed` — [UNSTABLE] temporary guardian notification carrying `{threadId, turnId, targetItemId, review, action?}` when guardian approval review resolves. This shape is expected to change soon.
- `item/autoApprovalReview/started` — [UNSTABLE] temporary guardian notification carrying `{threadId, turnId, targetItemId, parentToolItemId, review, action?}` when guardian approval review begins. This shape is expected to change soon.
- `item/autoApprovalReview/completed` — [UNSTABLE] temporary guardian notification carrying `{threadId, turnId, targetItemId, parentToolItemId, review, action?}` when guardian approval review resolves. This shape is expected to change soon.
`review` is [UNSTABLE] and currently has `{status, riskScore?, riskLevel?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `action` is the guardian action summary payload from core when available and is intended to support temporary standalone pending-review UI. `targetItemId` points at the reviewed item when app-server can attribute the review to a parent tool item, including attributed `network_access` reviews; otherwise it falls back to the guardian review id. These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the guardian app protocol is still being designed.
`review` is [UNSTABLE] and currently has `{status, riskScore?, riskLevel?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `action` is the guardian action summary payload from core when available and is intended to support temporary standalone pending-review UI. `targetItemId` is the stable guardian review id for matching started/completed notifications. `parentToolItemId` points at the reviewed parent tool item when app-server can attribute the review to one, including attributed `network_access` reviews. These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the guardian app protocol is still being designed.
There are additional item-specific events:
codex-rs/app-server/src/bespoke_event_handling.rs
+11 -1
View File
@@ -224,6 +224,7 @@ fn guardian_auto_approval_review_notification(
risk_level: assessment.risk_level.map(Into::into),
rationale: assessment.rationale.clone(),
};
let parent_tool_item_id = assessment.parent_tool_item_id.clone();
let target_item_id = assessment.id.clone();
match assessment.status {
codex_protocol::protocol::GuardianAssessmentStatus::InProgress => {
@@ -232,6 +233,7 @@ fn guardian_auto_approval_review_notification(
thread_id: conversation_id.to_string(),
turn_id,
target_item_id,
parent_tool_item_id,
review,
action: assessment.action.clone(),
},
@@ -245,6 +247,7 @@ fn guardian_auto_approval_review_notification(
thread_id: conversation_id.to_string(),
turn_id,
target_item_id,
parent_tool_item_id,
review,
action: assessment.action.clone(),
},
@@ -2919,6 +2922,7 @@ mod tests {
"turn-from-event",
&GuardianAssessmentEvent {
id: "item-1".to_string(),
parent_tool_item_id: None,
turn_id: String::new(),
status: codex_protocol::protocol::GuardianAssessmentStatus::InProgress,
risk_score: None,
@@ -2933,6 +2937,7 @@ mod tests {
assert_eq!(payload.thread_id, conversation_id.to_string());
assert_eq!(payload.turn_id, "turn-from-event");
assert_eq!(payload.target_item_id, "item-1");
assert_eq!(payload.parent_tool_item_id, None);
assert_eq!(
payload.review.status,
GuardianApprovalReviewStatus::InProgress
@@ -2958,6 +2963,7 @@ mod tests {
"turn-from-event",
&GuardianAssessmentEvent {
id: "item-2".to_string(),
parent_tool_item_id: None,
turn_id: "turn-from-assessment".to_string(),
status: codex_protocol::protocol::GuardianAssessmentStatus::Denied,
risk_score: Some(91),
@@ -2972,6 +2978,7 @@ mod tests {
assert_eq!(payload.thread_id, conversation_id.to_string());
assert_eq!(payload.turn_id, "turn-from-assessment");
assert_eq!(payload.target_item_id, "item-2");
assert_eq!(payload.parent_tool_item_id, None);
assert_eq!(payload.review.status, GuardianApprovalReviewStatus::Denied);
assert_eq!(payload.review.risk_score, Some(91));
assert_eq!(
@@ -2990,7 +2997,6 @@ mod tests {
let conversation_id = ThreadId::new();
let action = json!({
"tool": "network_access",
"parent_tool_item_id": "command-3",
"target": "api.openai.com:443",
});
let notification = guardian_auto_approval_review_notification(
@@ -2998,6 +3004,7 @@ mod tests {
"turn-from-event",
&GuardianAssessmentEvent {
id: "guardian-3".to_string(),
parent_tool_item_id: Some("command-3".to_string()),
turn_id: "turn-from-assessment".to_string(),
status: codex_protocol::protocol::GuardianAssessmentStatus::Aborted,
risk_score: None,
@@ -3012,6 +3019,7 @@ mod tests {
assert_eq!(payload.thread_id, conversation_id.to_string());
assert_eq!(payload.turn_id, "turn-from-assessment");
assert_eq!(payload.target_item_id, "guardian-3");
assert_eq!(payload.parent_tool_item_id.as_deref(), Some("command-3"));
assert_eq!(payload.review.status, GuardianApprovalReviewStatus::Aborted);
assert_eq!(payload.review.risk_score, None);
assert_eq!(payload.review.risk_level, None);
@@ -3034,6 +3042,7 @@ mod tests {
"turn-from-event",
&GuardianAssessmentEvent {
id: "guardian-4".to_string(),
parent_tool_item_id: None,
turn_id: "turn-from-assessment".to_string(),
status: codex_protocol::protocol::GuardianAssessmentStatus::Aborted,
risk_score: None,
@@ -3048,6 +3057,7 @@ mod tests {
assert_eq!(payload.thread_id, conversation_id.to_string());
assert_eq!(payload.turn_id, "turn-from-assessment");
assert_eq!(payload.target_item_id, "guardian-4");
assert_eq!(payload.parent_tool_item_id, None);
assert_eq!(payload.review.status, GuardianApprovalReviewStatus::Aborted);
assert_eq!(payload.review.risk_score, None);
assert_eq!(payload.review.risk_level, None);
codex-rs/core/src/codex_delegate_tests.rs
+1
View File
@@ -313,6 +313,7 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
assessment_event,
GuardianAssessmentEvent {
id: "command-item-1".to_string(),
parent_tool_item_id: None,
turn_id: parent_ctx.sub_id.clone(),
status: GuardianAssessmentStatus::InProgress,
risk_score: None,
codex-rs/core/src/guardian/approval_request.rs
+25 -23
View File
@@ -337,33 +337,18 @@ pub(crate) fn guardian_assessment_action_value(action: &GuardianApprovalRequest)
GuardianApprovalRequest::NetworkAccess {
id: _,
turn_id: _,
parent_tool_item_id,
target,
host,
protocol,
port,
} => {
let mut action = serde_json::Map::from_iter([
(
"tool".to_string(),
Value::String("network_access".to_string()),
),
("target".to_string(), Value::String(target.clone())),
("host".to_string(), Value::String(host.clone())),
(
"protocol".to_string(),
network_approval_protocol_value(*protocol),
),
("port".to_string(), Value::from(*port)),
]);
if let Some(parent_tool_item_id) = parent_tool_item_id {
action.insert(
"parent_tool_item_id".to_string(),
Value::String(parent_tool_item_id.clone()),
);
}
Value::Object(action)
}
..
} => serde_json::json!({
"tool": "network_access",
"target": target,
"host": host,
"protocol": network_approval_protocol_value(*protocol),
"port": port,
}),
GuardianApprovalRequest::McpToolCall {
server, tool_name, ..
} => serde_json::json!({
@@ -413,6 +398,23 @@ pub(crate) fn guardian_request_turn_id<'a>(
}
}
pub(crate) fn guardian_request_parent_tool_item_id(
request: &GuardianApprovalRequest,
) -> Option<&str> {
match request {
GuardianApprovalRequest::NetworkAccess {
parent_tool_item_id,
..
} => parent_tool_item_id.as_deref(),
GuardianApprovalRequest::Shell { .. }
| GuardianApprovalRequest::ExecCommand { .. }
| GuardianApprovalRequest::ApplyPatch { .. }
| GuardianApprovalRequest::McpToolCall { .. } => None,
#[cfg(unix)]
GuardianApprovalRequest::Execve { .. } => None,
}
}
pub(crate) fn format_guardian_action_pretty(
action: &GuardianApprovalRequest,
) -> serde_json::Result<String> {
codex-rs/core/src/guardian/review.rs
+7
View File
@@ -20,6 +20,7 @@ use super::GuardianApprovalRequest;
use super::GuardianAssessment;
use super::approval_request::guardian_assessment_action_value;
use super::approval_request::guardian_request_id;
use super::approval_request::guardian_request_parent_tool_item_id;
use super::approval_request::guardian_request_turn_id;
use super::prompt::build_guardian_prompt_items;
use super::prompt::guardian_output_schema;
@@ -81,12 +82,15 @@ async fn run_guardian_review(
) -> ReviewDecision {
let assessment_id = guardian_request_id(&request).to_string();
let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string();
let assessment_parent_tool_item_id =
guardian_request_parent_tool_item_id(&request).map(ToString::to_string);
let action_summary = guardian_assessment_action_value(&request);
session
.send_event(
turn.as_ref(),
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: assessment_id.clone(),
parent_tool_item_id: assessment_parent_tool_item_id.clone(),
turn_id: assessment_turn_id.clone(),
status: GuardianAssessmentStatus::InProgress,
risk_score: None,
@@ -106,6 +110,7 @@ async fn run_guardian_review(
turn.as_ref(),
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: assessment_id,
parent_tool_item_id: assessment_parent_tool_item_id.clone(),
turn_id: assessment_turn_id,
status: GuardianAssessmentStatus::Aborted,
risk_score: None,
@@ -156,6 +161,7 @@ async fn run_guardian_review(
turn.as_ref(),
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: assessment_id,
parent_tool_item_id: assessment_parent_tool_item_id.clone(),
turn_id: assessment_turn_id,
status: GuardianAssessmentStatus::Aborted,
risk_score: None,
@@ -192,6 +198,7 @@ async fn run_guardian_review(
turn.as_ref(),
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: assessment_id,
parent_tool_item_id: assessment_parent_tool_item_id,
turn_id: assessment_turn_id,
status,
risk_score: Some(assessment.risk_score),
codex-rs/core/src/guardian/tests.rs
+51 -2
View File
@@ -373,7 +373,7 @@ fn guardian_request_turn_id_prefers_network_access_owner_turn() {
}
#[test]
fn guardian_assessment_action_value_includes_network_access_parent_tool_item_id() {
fn guardian_assessment_action_value_omits_network_access_parent_tool_item_id() {
let network_access = GuardianApprovalRequest::NetworkAccess {
id: "network-1".to_string(),
turn_id: "owner-turn".to_string(),
@@ -392,7 +392,6 @@ fn guardian_assessment_action_value_includes_network_access_parent_tool_item_id(
"host": "example.com",
"protocol": "https",
"port": 443,
"parent_tool_item_id": "command-1",
})
);
}
@@ -441,6 +440,56 @@ async fn cancelled_guardian_review_emits_terminal_abort_without_warning() {
assert!(warnings.is_empty());
}
#[tokio::test]
async fn cancelled_network_guardian_review_preserves_parent_tool_item_id() {
let (session, turn, rx) = crate::codex::make_session_and_context_with_rx().await;
let cancel_token = CancellationToken::new();
cancel_token.cancel();
let decision = review_approval_request_with_cancel(
&session,
&turn,
GuardianApprovalRequest::NetworkAccess {
id: "network-1".to_string(),
turn_id: "owner-turn".to_string(),
parent_tool_item_id: Some("command-1".to_string()),
target: "https://example.com:443".to_string(),
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
port: 443,
},
None,
cancel_token,
)
.await;
assert_eq!(decision, ReviewDecision::Abort);
let mut guardian_events = Vec::new();
while let Ok(event) = rx.try_recv() {
if let EventMsg::GuardianAssessment(event) = event.msg {
guardian_events.push(event);
}
}
assert_eq!(
guardian_events
.into_iter()
.map(|event| (event.status, event.parent_tool_item_id))
.collect::<Vec<_>>(),
vec![
(
GuardianAssessmentStatus::InProgress,
Some("command-1".to_string()),
),
(
GuardianAssessmentStatus::Aborted,
Some("command-1".to_string()),
),
]
);
}
#[tokio::test]
async fn routes_approval_to_guardian_requires_auto_only_review_policy() {
let (_session, mut turn) = crate::codex::make_session_and_context().await;
codex-rs/protocol/src/approvals.rs
+5
View File
@@ -118,6 +118,11 @@ pub struct ExecApprovalRequestSkillMetadata {
pub struct GuardianAssessmentEvent {
/// Stable identifier for this guardian review lifecycle.
pub id: String,
/// Parent tool item identifier when this guardian review is attached to a
/// specific tool call.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub parent_tool_item_id: Option<String>,
/// Turn ID that this assessment belongs to.
/// Uses `#[serde(default)]` for backwards compatibility.
#[serde(default)]