mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Chore: remove response model check and rely on header model for downgrade (#12061)
### Summary Ensure that we use the model value from the response header only so that we are guaranteed with the correct slug name. We are no longer checking against the model value from response so that we are less likely to have false positive. There are two different treatments - for SSE we use the header from the response and for websocket we check top-level events.
This commit is contained in:
@@ -99,7 +99,9 @@ async fn response_model_field_mismatch_emits_model_rerouted_notification_v2_when
|
||||
"type": "response.created",
|
||||
"response": {
|
||||
"id": "resp-1",
|
||||
"model": SERVER_MODEL,
|
||||
"headers": {
|
||||
"OpenAI-Model": SERVER_MODEL
|
||||
}
|
||||
}
|
||||
}),
|
||||
responses::ev_assistant_message("msg-1", "Done"),
|
||||
|
||||
@@ -167,6 +167,7 @@ struct ResponseCompletedOutputTokensDetails {
|
||||
pub struct ResponsesStreamEvent {
|
||||
#[serde(rename = "type")]
|
||||
pub(crate) kind: String,
|
||||
headers: Option<Value>,
|
||||
response: Option<Value>,
|
||||
item: Option<Value>,
|
||||
delta: Option<String>,
|
||||
@@ -179,20 +180,27 @@ impl ResponsesStreamEvent {
|
||||
&self.kind
|
||||
}
|
||||
|
||||
/// Returns the effective model reported by the server, if present.
|
||||
///
|
||||
/// Precedence:
|
||||
/// 1. `response.headers` for standard Responses stream events.
|
||||
/// 2. top-level `headers` for websocket metadata events (for example
|
||||
/// `codex.response.metadata`).
|
||||
pub fn response_model(&self) -> Option<String> {
|
||||
self.response.as_ref().and_then(extract_server_model)
|
||||
}
|
||||
}
|
||||
let response_headers_model = self
|
||||
.response
|
||||
.as_ref()
|
||||
.and_then(|response| response.get("headers"))
|
||||
.and_then(header_openai_model_value_from_json);
|
||||
|
||||
fn extract_server_model(value: &Value) -> Option<String> {
|
||||
value
|
||||
.get("model")
|
||||
.and_then(json_value_as_string)
|
||||
.or_else(|| {
|
||||
value
|
||||
.get("headers")
|
||||
.and_then(header_openai_model_value_from_json)
|
||||
})
|
||||
match response_headers_model {
|
||||
Some(model) => Some(model),
|
||||
None => self
|
||||
.headers
|
||||
.as_ref()
|
||||
.and_then(header_openai_model_value_from_json),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn header_openai_model_value_from_json(value: &Value) -> Option<String> {
|
||||
@@ -963,7 +971,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn process_sse_emits_server_model_from_response_payload() {
|
||||
async fn process_sse_ignores_response_model_field_in_payload() {
|
||||
let events = run_sse(vec![
|
||||
json!({
|
||||
"type": "response.created",
|
||||
@@ -982,14 +990,10 @@ mod tests {
|
||||
])
|
||||
.await;
|
||||
|
||||
assert_eq!(events.len(), 3);
|
||||
assert_eq!(events.len(), 2);
|
||||
assert_matches!(&events[0], ResponseEvent::Created);
|
||||
assert_matches!(
|
||||
&events[0],
|
||||
ResponseEvent::ServerModel(model) if model == CYBER_RESTRICTED_MODEL_FOR_TESTS
|
||||
);
|
||||
assert_matches!(&events[1], ResponseEvent::Created);
|
||||
assert_matches!(
|
||||
&events[2],
|
||||
&events[1],
|
||||
ResponseEvent::Completed {
|
||||
response_id,
|
||||
token_usage: None,
|
||||
@@ -1035,43 +1039,41 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn process_sse_emits_server_model_again_when_response_model_changes() {
|
||||
let events = run_sse(vec![
|
||||
json!({
|
||||
"type": "response.created",
|
||||
"response": {
|
||||
"id": "resp-1",
|
||||
"model": "gpt-5.2-codex"
|
||||
}
|
||||
}),
|
||||
json!({
|
||||
"type": "response.completed",
|
||||
"response": {
|
||||
"id": "resp-1",
|
||||
"model": "gpt-5.3-codex"
|
||||
}
|
||||
}),
|
||||
])
|
||||
.await;
|
||||
#[test]
|
||||
fn responses_stream_event_response_model_reads_top_level_headers() {
|
||||
let ev: ResponsesStreamEvent = serde_json::from_value(json!({
|
||||
"type": "codex.response.metadata",
|
||||
"headers": {
|
||||
"openai-model": CYBER_RESTRICTED_MODEL_FOR_TESTS,
|
||||
}
|
||||
}))
|
||||
.expect("expected event to deserialize");
|
||||
|
||||
assert_eq!(events.len(), 4);
|
||||
assert_matches!(
|
||||
&events[0],
|
||||
ResponseEvent::ServerModel(model) if model == "gpt-5.2-codex"
|
||||
assert_eq!(
|
||||
ev.response_model().as_deref(),
|
||||
Some(CYBER_RESTRICTED_MODEL_FOR_TESTS)
|
||||
);
|
||||
assert_matches!(&events[1], ResponseEvent::Created);
|
||||
assert_matches!(
|
||||
&events[2],
|
||||
ResponseEvent::ServerModel(model) if model == "gpt-5.3-codex"
|
||||
);
|
||||
assert_matches!(
|
||||
&events[3],
|
||||
ResponseEvent::Completed {
|
||||
response_id,
|
||||
token_usage: None,
|
||||
can_append: false
|
||||
} if response_id == "resp-1"
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responses_stream_event_response_model_prefers_response_headers() {
|
||||
let ev: ResponsesStreamEvent = serde_json::from_value(json!({
|
||||
"type": "response.created",
|
||||
"headers": {
|
||||
"openai-model": "top-level-model"
|
||||
},
|
||||
"response": {
|
||||
"id": "resp-1",
|
||||
"headers": {
|
||||
"openai-model": CYBER_RESTRICTED_MODEL_FOR_TESTS
|
||||
}
|
||||
}
|
||||
}))
|
||||
.expect("expected event to deserialize");
|
||||
|
||||
assert_eq!(
|
||||
ev.response_model().as_deref(),
|
||||
Some(CYBER_RESTRICTED_MODEL_FOR_TESTS)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -2440,7 +2440,9 @@ impl Session {
|
||||
server_model: String,
|
||||
) -> bool {
|
||||
let requested_model = turn_context.model_info.slug.clone();
|
||||
if server_model == requested_model {
|
||||
let server_model_normalized = server_model.to_ascii_lowercase();
|
||||
let requested_model_normalized = requested_model.to_ascii_lowercase();
|
||||
if server_model_normalized == requested_model_normalized {
|
||||
info!("server reported model {server_model} (matches requested model)");
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -121,7 +121,9 @@ async fn response_model_field_mismatch_emits_warning_when_header_matches_request
|
||||
"type": "response.created",
|
||||
"response": {
|
||||
"id": "resp-1",
|
||||
"model": SERVER_MODEL,
|
||||
"headers": {
|
||||
"OpenAI-Model": SERVER_MODEL
|
||||
}
|
||||
}
|
||||
}),
|
||||
core_test_support::responses::ev_completed("resp-1"),
|
||||
@@ -250,3 +252,58 @@ async fn openai_model_header_mismatch_only_emits_one_warning_per_turn() -> Resul
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn openai_model_header_casing_only_mismatch_does_not_warn() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let requested_header = REQUESTED_MODEL.to_ascii_uppercase();
|
||||
let response = sse_response(sse_completed("resp-1"))
|
||||
.insert_header("OpenAI-Model", requested_header.as_str());
|
||||
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 {
|
||||
items: vec![UserInput::Text {
|
||||
text: "trigger casing check".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: REQUESTED_MODEL.to_string(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut reroute_count = 0;
|
||||
let mut warning_count = 0;
|
||||
loop {
|
||||
let event = wait_for_event(&test.codex, |_| true).await;
|
||||
match event {
|
||||
EventMsg::ModelReroute(_) => reroute_count += 1,
|
||||
EventMsg::Warning(warning)
|
||||
if warning
|
||||
.message
|
||||
.contains("flagged for potentially high-risk cyber activity") =>
|
||||
{
|
||||
warning_count += 1;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
assert_eq!(reroute_count, 0);
|
||||
assert_eq!(warning_count, 0);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user