mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add safety check notification and error handling (#19055)
Adds a new app-server notification that fires when a user account has been flagged for potential safety reasons.
This commit is contained in:
committed by
GitHub
Unverified
parent
02170996e6
commit
bbff4ee61a
@@ -331,6 +331,8 @@ use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::McpServerRefreshConfig;
|
||||
use codex_protocol::protocol::ModelRerouteEvent;
|
||||
use codex_protocol::protocol::ModelRerouteReason;
|
||||
use codex_protocol::protocol::ModelVerification;
|
||||
use codex_protocol::protocol::ModelVerificationEvent;
|
||||
use codex_protocol::protocol::NetworkApprovalContext;
|
||||
use codex_protocol::protocol::NonSteerableTurnKind;
|
||||
use codex_protocol::protocol::Op;
|
||||
@@ -2385,6 +2387,18 @@ impl Session {
|
||||
true
|
||||
}
|
||||
|
||||
pub(crate) async fn emit_model_verification(
|
||||
self: &Arc<Self>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
verifications: Vec<ModelVerification>,
|
||||
) {
|
||||
self.send_event(
|
||||
turn_context,
|
||||
EventMsg::ModelVerification(ModelVerificationEvent { verifications }),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(crate) async fn replace_history(
|
||||
&self,
|
||||
items: Vec<ResponseItem>,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::turn_context::image_generation_tool_auth_allowed;
|
||||
use super::*;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
|
||||
/// Spawn a review thread using the given prompt.
|
||||
pub(super) async fn spawn_review_thread(
|
||||
@@ -140,6 +141,8 @@ pub(super) async fn spawn_review_thread(
|
||||
turn_metadata_state,
|
||||
turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.outcome.clone()),
|
||||
turn_timing_state: Arc::new(TurnTimingState::default()),
|
||||
server_model_warning_emitted: AtomicBool::new(false),
|
||||
model_verification_emitted: AtomicBool::new(false),
|
||||
};
|
||||
|
||||
// Seed the child task with the review prompt as the initial user message.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use crate::SkillInjections;
|
||||
use crate::SkillLoadOutcome;
|
||||
@@ -364,7 +365,6 @@ pub(crate) async fn run_turn(
|
||||
// Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains
|
||||
// many turns, from the perspective of the user, it is a single turn.
|
||||
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let mut server_model_warning_emitted_for_turn = false;
|
||||
|
||||
// `ModelClientSession` is turn-scoped and caches WebSocket + sticky routing state, so we reuse
|
||||
// one instance across retries within this turn.
|
||||
@@ -455,7 +455,6 @@ pub(crate) async fn run_turn(
|
||||
sampling_request_input,
|
||||
&explicitly_enabled_connectors,
|
||||
skills_outcome,
|
||||
&mut server_model_warning_emitted_for_turn,
|
||||
cancellation_token.child_token(),
|
||||
)
|
||||
.await
|
||||
@@ -1022,7 +1021,6 @@ async fn run_sampling_request(
|
||||
input: Vec<ResponseItem>,
|
||||
explicitly_enabled_connectors: &HashSet<String>,
|
||||
skills_outcome: Option<&SkillLoadOutcome>,
|
||||
server_model_warning_emitted_for_turn: &mut bool,
|
||||
cancellation_token: CancellationToken,
|
||||
) -> CodexResult<SamplingRequestResult> {
|
||||
let router = built_tools(
|
||||
@@ -1076,7 +1074,6 @@ async fn run_sampling_request(
|
||||
client_session,
|
||||
turn_metadata_header,
|
||||
Arc::clone(&turn_diff_tracker),
|
||||
server_model_warning_emitted_for_turn,
|
||||
&prompt,
|
||||
cancellation_token.child_token(),
|
||||
)
|
||||
@@ -1490,6 +1487,7 @@ pub(super) fn realtime_text_for_event(msg: &EventMsg) -> Option<String> {
|
||||
| EventMsg::RealtimeConversationRealtime(_)
|
||||
| EventMsg::RealtimeConversationClosed(_)
|
||||
| EventMsg::ModelReroute(_)
|
||||
| EventMsg::ModelVerification(_)
|
||||
| EventMsg::ContextCompacted(_)
|
||||
| EventMsg::ThreadRolledBack(_)
|
||||
| EventMsg::TurnStarted(_)
|
||||
@@ -1866,7 +1864,6 @@ async fn try_run_sampling_request(
|
||||
client_session: &mut ModelClientSession,
|
||||
turn_metadata_header: Option<&str>,
|
||||
turn_diff_tracker: SharedTurnDiffTracker,
|
||||
server_model_warning_emitted_for_turn: &mut bool,
|
||||
prompt: &Prompt,
|
||||
cancellation_token: CancellationToken,
|
||||
) -> CodexResult<SamplingRequestResult> {
|
||||
@@ -2099,12 +2096,25 @@ async fn try_run_sampling_request(
|
||||
}
|
||||
}
|
||||
ResponseEvent::ServerModel(server_model) => {
|
||||
if !*server_model_warning_emitted_for_turn
|
||||
if !turn_context
|
||||
.server_model_warning_emitted
|
||||
.load(Ordering::Relaxed)
|
||||
&& sess
|
||||
.maybe_warn_on_server_model_mismatch(&turn_context, server_model)
|
||||
.await
|
||||
{
|
||||
*server_model_warning_emitted_for_turn = true;
|
||||
turn_context
|
||||
.server_model_warning_emitted
|
||||
.store(true, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
ResponseEvent::ModelVerifications(verifications) => {
|
||||
if !turn_context
|
||||
.model_verification_emitted
|
||||
.swap(true, Ordering::Relaxed)
|
||||
{
|
||||
sess.emit_model_verification(&turn_context, verifications)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
ResponseEvent::ServerReasoningIncluded(included) => {
|
||||
|
||||
@@ -3,6 +3,8 @@ use codex_model_provider::SharedModelProvider;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
pub(super) fn image_generation_tool_auth_allowed(auth_manager: Option<&AuthManager>) -> bool {
|
||||
matches!(
|
||||
@@ -82,6 +84,8 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) turn_metadata_state: Arc<TurnMetadataState>,
|
||||
pub(crate) turn_skills: TurnSkillsContext,
|
||||
pub(crate) turn_timing_state: Arc<TurnTimingState>,
|
||||
pub(crate) server_model_warning_emitted: AtomicBool,
|
||||
pub(crate) model_verification_emitted: AtomicBool,
|
||||
}
|
||||
impl TurnContext {
|
||||
pub(crate) fn permission_profile(&self) -> PermissionProfile {
|
||||
@@ -216,6 +220,12 @@ impl TurnContext {
|
||||
turn_metadata_state: self.turn_metadata_state.clone(),
|
||||
turn_skills: self.turn_skills.clone(),
|
||||
turn_timing_state: Arc::clone(&self.turn_timing_state),
|
||||
server_model_warning_emitted: AtomicBool::new(
|
||||
self.server_model_warning_emitted.load(Ordering::Relaxed),
|
||||
),
|
||||
model_verification_emitted: AtomicBool::new(
|
||||
self.model_verification_emitted.load(Ordering::Relaxed),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -469,6 +479,8 @@ impl Session {
|
||||
turn_metadata_state,
|
||||
turn_skills: TurnSkillsContext::new(skills_outcome),
|
||||
turn_timing_state: Arc::new(TurnTimingState::default()),
|
||||
server_model_warning_emitted: AtomicBool::new(false),
|
||||
model_verification_emitted: AtomicBool::new(false),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -143,6 +143,7 @@ fn response_event_records_turn_ttft(event: &ResponseEvent) -> bool {
|
||||
| ResponseEvent::ReasoningContentDelta { .. } => true,
|
||||
ResponseEvent::Created
|
||||
| ResponseEvent::ServerModel(_)
|
||||
| ResponseEvent::ModelVerifications(_)
|
||||
| ResponseEvent::ServerReasoningIncluded(_)
|
||||
| ResponseEvent::ToolCallInputDelta { .. }
|
||||
| ResponseEvent::Completed { .. }
|
||||
|
||||
@@ -632,6 +632,17 @@ pub fn ev_response_created(id: &str) -> Value {
|
||||
})
|
||||
}
|
||||
|
||||
pub fn ev_model_verification_metadata(id: &str, verifications: Vec<&str>) -> Value {
|
||||
serde_json::json!({
|
||||
"type": "response.metadata",
|
||||
"sequence_number": 1,
|
||||
"response_id": id,
|
||||
"metadata": {
|
||||
"openai_verification_recommendation": verifications,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub fn ev_completed_with_tokens(id: &str, total_tokens: i64) -> Value {
|
||||
serde_json::json!({
|
||||
"type": "response.completed",
|
||||
|
||||
@@ -2,13 +2,16 @@ use anyhow::Result;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::CodexErrorInfo;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ModelRerouteReason;
|
||||
use codex_protocol::protocol::ModelVerification;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_model_verification_metadata;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_response_once;
|
||||
use core_test_support::responses::mount_response_sequence;
|
||||
@@ -20,9 +23,14 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use wiremock::ResponseTemplate;
|
||||
|
||||
const SERVER_MODEL: &str = "gpt-5.2";
|
||||
const REQUESTED_MODEL: &str = "gpt-5.3-codex";
|
||||
const TRUSTED_ACCESS_FOR_CYBER_VERIFICATION: &str = "trusted_access_for_cyber";
|
||||
|
||||
const CYBER_POLICY_MESSAGE: &str =
|
||||
"This request has been flagged for potentially high-risk cyber activity.";
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn openai_model_header_mismatch_emits_warning_event_and_warning_item() -> Result<()> {
|
||||
@@ -113,6 +121,57 @@ async fn openai_model_header_mismatch_emits_warning_event_and_warning_item() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn cyber_policy_response_emits_typed_error_without_retry() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let response = ResponseTemplate::new(400).set_body_json(serde_json::json!({
|
||||
"error": {
|
||||
"message": CYBER_POLICY_MESSAGE,
|
||||
"type": "invalid_request",
|
||||
"param": null,
|
||||
"code": "cyber_policy"
|
||||
}
|
||||
}));
|
||||
let mock = mount_response_once(&server, response).await;
|
||||
|
||||
let mut builder = test_codex().with_model(REQUESTED_MODEL);
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
environments: None,
|
||||
items: vec![UserInput::Text {
|
||||
text: "trigger cyber policy error".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: REQUESTED_MODEL.to_string(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: None,
|
||||
service_tier: None,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let error = wait_for_event(&test.codex, |event| matches!(event, EventMsg::Error(_))).await;
|
||||
let EventMsg::Error(error) = error else {
|
||||
panic!("expected error event");
|
||||
};
|
||||
assert_eq!(error.message, CYBER_POLICY_MESSAGE);
|
||||
assert_eq!(error.codex_error_info, Some(CodexErrorInfo::CyberPolicy));
|
||||
|
||||
mock.single_request();
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn response_model_field_mismatch_emits_warning_when_header_matches_requested() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -318,3 +377,150 @@ async fn openai_model_header_casing_only_mismatch_does_not_warn() -> Result<()>
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_verification_emits_structured_event_without_reroute_or_warning() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let response = sse_response(sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_model_verification_metadata("resp-1", vec![TRUSTED_ACCESS_FOR_CYBER_VERIFICATION]),
|
||||
core_test_support::responses::ev_completed("resp-1"),
|
||||
]));
|
||||
let _mock = mount_response_once(&server, response).await;
|
||||
|
||||
let mut builder = test_codex().with_model(REQUESTED_MODEL);
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
environments: None,
|
||||
items: vec![UserInput::Text {
|
||||
text: "trigger model verification".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: REQUESTED_MODEL.to_string(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: None,
|
||||
service_tier: None,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut verification_count = 0;
|
||||
let mut reroute_count = 0;
|
||||
let mut warning_count = 0;
|
||||
let mut warning_item_count = 0;
|
||||
loop {
|
||||
let event = wait_for_event(&test.codex, |_| true).await;
|
||||
match event {
|
||||
EventMsg::ModelVerification(event) => {
|
||||
assert_eq!(
|
||||
event.verifications,
|
||||
vec![ModelVerification::TrustedAccessForCyber]
|
||||
);
|
||||
verification_count += 1;
|
||||
}
|
||||
EventMsg::Warning(_) => warning_count += 1,
|
||||
EventMsg::ModelReroute(_) => reroute_count += 1,
|
||||
EventMsg::RawResponseItem(raw)
|
||||
if matches!(
|
||||
&raw.item,
|
||||
ResponseItem::Message { content, .. }
|
||||
if content.iter().any(|item| matches!(
|
||||
item,
|
||||
ContentItem::InputText { text } if text.starts_with("Warning: ")
|
||||
))
|
||||
) =>
|
||||
{
|
||||
warning_item_count += 1;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
assert_eq!(verification_count, 1);
|
||||
assert_eq!(reroute_count, 0);
|
||||
assert_eq!(warning_count, 0);
|
||||
assert_eq!(warning_item_count, 0);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_verification_only_emits_once_per_turn() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let tool_args = serde_json::json!({
|
||||
"command": "echo hello",
|
||||
"timeout_ms": 1_000
|
||||
});
|
||||
|
||||
let first_response = sse_response(sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(
|
||||
"call-1",
|
||||
"shell_command",
|
||||
&serde_json::to_string(&tool_args)?,
|
||||
),
|
||||
ev_model_verification_metadata("resp-1", vec![TRUSTED_ACCESS_FOR_CYBER_VERIFICATION]),
|
||||
core_test_support::responses::ev_completed("resp-1"),
|
||||
]));
|
||||
let second_response = sse_response(sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_model_verification_metadata("resp-2", vec![TRUSTED_ACCESS_FOR_CYBER_VERIFICATION]),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
core_test_support::responses::ev_completed("resp-2"),
|
||||
]));
|
||||
let _mock = mount_response_sequence(&server, vec![first_response, second_response]).await;
|
||||
|
||||
let mut builder = test_codex().with_model(REQUESTED_MODEL);
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
environments: None,
|
||||
items: vec![UserInput::Text {
|
||||
text: "trigger follow-up model verification".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: REQUESTED_MODEL.to_string(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: None,
|
||||
service_tier: None,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut verification_count = 0;
|
||||
loop {
|
||||
let event = wait_for_event(&test.codex, |_| true).await;
|
||||
match event {
|
||||
EventMsg::ModelVerification(_) => verification_count += 1,
|
||||
EventMsg::Warning(warning) if warning.message.contains("high-risk cyber activity") => {
|
||||
panic!("model verification should not emit a warning event");
|
||||
}
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
assert_eq!(verification_count, 1);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user