Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/analytics/src/analytics_client_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ fn sample_guardian_review_completed(
risk_level: None,
user_authorization: None,
rationale: None,
denial_kind: None,
},
action: GuardianApprovalReviewAction::Command {
source: AppServerGuardianCommandSource::Shell,
Expand Down
18 changes: 18 additions & 0 deletions codex-rs/app-server-protocol/schema/json/ServerNotification.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions codex-rs/app-server-protocol/schema/typescript/v2/index.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions codex-rs/app-server-protocol/src/protocol/item_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ pub fn guardian_auto_approval_review_notification(
risk_level: assessment.risk_level.map(Into::into),
user_authorization: assessment.user_authorization.map(Into::into),
rationale: assessment.rationale.clone(),
denial_kind: assessment.denial_kind.map(Into::into),
};
let action = assessment.action.clone().into();
match assessment.status {
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/app-server-protocol/src/protocol/thread_history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2324,6 +2324,7 @@ mod tests {
risk_level: None,
user_authorization: None,
rationale: None,
denial_kind: None,
decision_source: None,
action: serde_json::from_value(serde_json::json!({
"type": "command",
Expand All @@ -2343,6 +2344,7 @@ mod tests {
risk_level: Some(codex_protocol::protocol::GuardianRiskLevel::High),
user_authorization: Some(codex_protocol::protocol::GuardianUserAuthorization::Low),
rationale: Some("Would delete user data.".into()),
denial_kind: Some(codex_protocol::protocol::GuardianDenialKind::Soft),
decision_source: Some(
codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
),
Expand Down Expand Up @@ -2410,6 +2412,7 @@ mod tests {
risk_level: None,
user_authorization: None,
rationale: None,
denial_kind: None,
decision_source: None,
action: serde_json::from_value(serde_json::json!({
"type": "execve",
Expand Down
20 changes: 20 additions & 0 deletions codex-rs/app-server-protocol/src/protocol/v2/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use codex_experimental_api_macros::ExperimentalApi;
use codex_protocol::approvals::GuardianAssessmentAction as CoreGuardianAssessmentAction;
use codex_protocol::approvals::GuardianAssessmentDecisionSource as CoreGuardianAssessmentDecisionSource;
use codex_protocol::approvals::GuardianCommandSource as CoreGuardianCommandSource;
use codex_protocol::approvals::GuardianDenialKind as CoreGuardianDenialKind;
use codex_protocol::items::AgentMessageContent as CoreAgentMessageContent;
use codex_protocol::items::McpToolCallStatus as CoreMcpToolCallStatus;
use codex_protocol::items::TurnItem as CoreTurnItem;
Expand Down Expand Up @@ -469,6 +470,24 @@ impl From<CoreGuardianUserAuthorization> for GuardianUserAuthorization {
}
}

#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "lowercase")]
#[ts(export_to = "v2/")]
/// [UNSTABLE] Whether a denied action can be explicitly approved for one retry.
pub enum GuardianDenialKind {
Soft,
Denial,
}

impl From<CoreGuardianDenialKind> for GuardianDenialKind {
fn from(value: CoreGuardianDenialKind) -> Self {
match value {
CoreGuardianDenialKind::Soft => Self::Soft,
CoreGuardianDenialKind::Denial => Self::Denial,
}
}
}

/// [UNSTABLE] Temporary approval auto-review payload used by
/// `item/autoApprovalReview/*` notifications. This shape is expected to change
/// soon.
Expand All @@ -480,6 +499,7 @@ pub struct GuardianApprovalReview {
pub risk_level: Option<GuardianRiskLevel>,
pub user_authorization: Option<GuardianUserAuthorization>,
pub rationale: Option<String>,
pub denial_kind: Option<GuardianDenialKind>,
}

#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/app-server-protocol/src/protocol/v2/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ impl From<CoreAskForApproval> for AskForApproval {
pub enum ApprovalsReviewer {
#[serde(rename = "user")]
User,
#[serde(rename = "guardian_subagent", alias = "auto_review")]
#[serde(rename = "auto_review", alias = "guardian_subagent")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve serialized approvals reviewer compatibility

This changes app-server v2 responses that serialize ApprovalsReviewer::AutoReview from the existing "guardian_subagent" wire value to "auto_review"; inputs remain backward-compatible via the alias, but older integrations that compare or deserialize the previous response literal from config/thread settings APIs will now see a different value. The repo review rules explicitly call out app-server API breaking changes (AGENTS.md lines 92-99), so keep output compatibility or version/gate the new canonical serialization.

Useful? React with 👍 / 👎.

AutoReview,
}

Expand Down
3 changes: 2 additions & 1 deletion codex-rs/app-server-protocol/src/protocol/v2/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ fn approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagen
);
assert_eq!(
serde_json::to_string(&ApprovalsReviewer::AutoReview).expect("serialize reviewer"),
"\"guardian_subagent\""
"\"auto_review\""
);

for value in ["user", "auto_review", "guardian_subagent"] {
Expand Down Expand Up @@ -2198,6 +2198,7 @@ fn automatic_approval_review_deserializes_aborted_status() {
risk_level: None,
user_authorization: None,
rationale: None,
denial_kind: None,
}
);
}
Expand Down
1 change: 1 addition & 0 deletions codex-rs/app-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,7 @@ All items emit shared lifecycle events:
- `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 auto-review notification carrying `{threadId, turnId, targetItemId, review, action}` when approval auto-review begins. This shape is expected to change soon.
- `item/autoApprovalReview/completed` — [UNSTABLE] temporary auto-review notification carrying `{threadId, turnId, targetItemId, review, action}` when approval auto-review resolves. This shape is expected to change soon.
For denied reviews, `review.denialKind` is `"soft"` when the exact action is eligible for one explicit retry and `"denial"` when it is not user-overridable. A null `denialKind` indicates an operational review failure rather than a policy denial.

`review` is [UNSTABLE] and currently has `{status, riskLevel?, userAuthorization?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `riskLevel` is one of `"low"`, `"medium"`, `"high"`, or `"critical"` when present. `userAuthorization` is one of `"unknown"`, `"low"`, `"medium"`, or `"high"` when present. `action` is a tagged union with `type: "command" | "execve" | "applyPatch" | "networkAccess" | "mcpToolCall"`. Command-like actions include a `source` discriminator (`"shell"` or `"unifiedExec"`). These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the auto-review app protocol is still being designed.

Expand Down
5 changes: 5 additions & 0 deletions codex-rs/app-server/src/bespoke_event_handling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2280,6 +2280,8 @@ mod tests {
risk_level,
user_authorization,
rationale,
denial_kind: matches!(status, GuardianAssessmentStatus::Denied)
.then_some(codex_protocol::protocol::GuardianDenialKind::Soft),
decision_source: if matches!(status, GuardianAssessmentStatus::InProgress) {
None
} else {
Expand Down Expand Up @@ -2346,6 +2348,7 @@ mod tests {
risk_level: None,
user_authorization: None,
rationale: None,
denial_kind: None,
decision_source: None,
action: action.clone(),
},
Expand Down Expand Up @@ -2392,6 +2395,7 @@ mod tests {
risk_level: Some(codex_protocol::protocol::GuardianRiskLevel::High),
user_authorization: Some(codex_protocol::protocol::GuardianUserAuthorization::Low),
rationale: Some("too risky".to_string()),
denial_kind: Some(codex_protocol::protocol::GuardianDenialKind::Soft),
decision_source: Some(
codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
),
Expand Down Expand Up @@ -2446,6 +2450,7 @@ mod tests {
risk_level: None,
user_authorization: None,
rationale: None,
denial_kind: None,
decision_source: Some(
codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
),
Expand Down
10 changes: 8 additions & 2 deletions codex-rs/app-server/src/request_processors/thread_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1756,8 +1756,14 @@ impl ThreadRequestProcessor {
params: ThreadApproveGuardianDeniedActionParams,
) -> Result<ThreadApproveGuardianDeniedActionResponse, JSONRPCErrorError> {
let ThreadApproveGuardianDeniedActionParams { thread_id, event } = params;
let event = serde_json::from_value(event)
.map_err(|err| invalid_request(format!("invalid Guardian denial event: {err}")))?;
let event: codex_protocol::protocol::GuardianAssessmentEvent =
serde_json::from_value(event)
.map_err(|err| invalid_request(format!("invalid Guardian denial event: {err}")))?;
if !event.is_explicit_retry_eligible() {
return Err(invalid_request(
"Guardian denial is not eligible for an explicit retry",
));
}
let (_, thread) = self.load_thread(&thread_id).await?;

self.submit_core_op(
Expand Down
Loading
Loading