Avoid redundant model catalog lookups in reused Guardian reviewers (#45933)

## Why

Reusing a Guardian reviewer could trigger a model catalog refresh during the previous-model compaction check, even when the model was unchanged and no compaction compatibility change required compaction.

## What changed

Return early from the previous-model compaction check for Guardian sessions when the model slug is unchanged and the compaction compatibility hashes do not require compaction.

## Testing

Add a regression test with an expired model catalog cache that verifies two approvals reuse the same Guardian reviewer, both reviews and the parent turn complete, and no additional model catalog request occurs.

GitOrigin-RevId: cc1cdc5f89c6f9ec03f1bedfb78cad9780825733

codex-rs/core/src/session/turn.rs

@@ -1321,6 +1321,12 @@ async fn maybe_run_previous_model_inline_compact(
         turn_context.model_info().comp_hash.as_deref(),
     );
     let previous_model = previous_turn_settings.model;
+    if crate::guardian::is_basic_session_source(&turn_context.session_source)
+        && !should_compact_for_comp_hash_change
+        && previous_model == turn_context.model_info().slug
+    {
+        return Ok(());
+    }
     let previous_model_turn_context = Arc::new(
         turn_context
             .with_model(previous_model.clone(), &sess.services.models_manager)

codex-rs/core/tests/suite/models_cache_ttl.rs

@@ -7,9 +7,11 @@ use anyhow::Result;
 use chrono::DateTime;
 use chrono::TimeZone;
 use chrono::Utc;
+use codex_core::config::Constrained;
 use codex_login::CodexAuth;
 use codex_models_manager::client_version_to_whole;
 use codex_models_manager::manager::RefreshStrategy;
+use codex_protocol::config_types::ApprovalsReviewer;
 use codex_protocol::config_types::CollaborationMode;
 use codex_protocol::config_types::ModeKind;
 use codex_protocol::config_types::ReasoningSummary;
@@ -24,15 +26,19 @@ use codex_protocol::openai_models::ReasoningEffort;
 use codex_protocol::openai_models::ReasoningEffortPreset;
 use codex_protocol::openai_models::TruncationPolicyConfig;
 use codex_protocol::openai_models::default_input_modalities;
+use codex_protocol::protocol::AskForApproval;
 use codex_protocol::protocol::EventMsg;
 use codex_protocol::protocol::ThreadSettingsOverrides;
 use codex_protocol::user_input::UserInput;
 use core_test_support::responses;
 use core_test_support::responses::ev_assistant_message;
 use core_test_support::responses::ev_completed;
+use core_test_support::responses::ev_function_call;
 use core_test_support::responses::ev_response_created;
+use core_test_support::responses::mount_sse_sequence;
 use core_test_support::responses::sse;
 use core_test_support::responses::sse_response;
+use core_test_support::skip_if_no_network;
 use core_test_support::test_codex::test_codex;
 use core_test_support::test_codex::turn_permission_fields;
 use core_test_support::wait_for_event;
@@ -49,6 +55,106 @@ const VERSIONED_MODEL: &str = "codex-test-versioned";
 const MISSING_VERSION_MODEL: &str = "codex-test-missing-version";
 const DIFFERENT_VERSION_MODEL: &str = "codex-test-different-version";
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn guardian_reused_reviewer_avoids_stale_catalog_lookup() -> Result<()> {
+    skip_if_no_network!(Ok(()));
+
+    let server = MockServer::start().await;
+    let bundled = codex_models_manager::bundled_models_response()?;
+    let catalog = ModelsResponse {
+        models: bundled
+            .models
+            .into_iter()
+            .filter(|model| ["gpt-5.4", "codex-auto-review"].contains(&model.slug.as_str()))
+            .collect(),
+    };
+    assert_eq!(catalog.models.len(), 2);
+    let initial_models =
+        responses::mount_models_once_with_etag(&server, catalog.clone(), ETAG).await;
+
+    let mut builder = test_codex()
+        .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
+        .with_model("gpt-5.4");
+    builder = builder.with_config(|config| {
+        config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
+        config.approvals_reviewer = ApprovalsReviewer::AutoReview;
+        config.model_provider.request_max_retries = Some(0);
+    });
+    let test = builder.build_with_auto_env(&server).await?;
+    test.thread_manager
+        .get_models_manager()
+        .list_models(
+            RefreshStrategy::OnlineIfUncached,
+            codex_core::test_support::default_http_client_factory(),
+        )
+        .await;
+    assert_eq!(initial_models.requests().len(), 1);
+    rewrite_cache_timestamp(
+        &test.config.codex_home.join(CACHE_FILE),
+        Utc::now() - chrono::Duration::hours(1),
+    )
+    .await?;
+    let repeat_models = responses::mount_models_once_with_etag(&server, catalog, ETAG).await;
+    let deny = r#"{"risk_level":"high","user_authorization":"low","outcome":"deny"}"#;
+    let mut sequence = Vec::new();
+    for index in 1..=2 {
+        let parent = format!("parent-{index}");
+        let guardian = format!("guardian-{index}");
+        sequence.push(sse(vec![
+            ev_response_created(&parent),
+            ev_function_call(
+                &format!("call-{index}"),
+                "exec_command",
+                r#"{"cmd":"true","sandbox_permissions":"require_escalated","justification":"Check an action before running it."}"#,
+            ),
+            ev_completed(&parent),
+        ]));
+        sequence.push(sse(vec![
+            ev_response_created(&guardian),
+            ev_assistant_message(&guardian, deny),
+            ev_completed(&guardian),
+        ]));
+    }
+    sequence.push(sse(vec![
+        ev_response_created("parent-done"),
+        ev_assistant_message("parent-done", "finished"),
+        ev_completed("parent-done"),
+    ]));
+    let response_mock = mount_sse_sequence(&server, sequence).await;
+    test.submit_text_turn("Review both shell requests.").await?;
+
+    let requests = response_mock.requests();
+    assert_eq!(
+        requests.len(),
+        5,
+        "both reviews and the parent turn must finish"
+    );
+    let guardian_requests = requests
+        .iter()
+        .filter(|request| {
+            request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian")
+        })
+        .collect::<Vec<_>>();
+    assert_eq!(guardian_requests.len(), 2);
+    for request in &guardian_requests {
+        assert_eq!(
+            request.body_json()["model"].as_str(),
+            Some("codex-auto-review")
+        );
+    }
+    let guardian_thread = guardian_requests[0].body_json()["client_metadata"]["thread_id"]
+        .as_str()
+        .expect("Guardian reviewer thread id")
+        .to_string();
+    assert_eq!(
+        guardian_requests[1].body_json()["client_metadata"]["thread_id"].as_str(),
+        Some(guardian_thread.as_str()),
+        "the second approval must reuse the Guardian reviewer"
+    );
+    assert_eq!(repeat_models.requests().len(), 0);
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn renews_cache_ttl_on_matching_models_etag() -> Result<()> {
     let server = MockServer::start().await;