core: simplify guardian trunk reuse matching

Co-authored-by: Codex <noreply@openai.com>

codex-rs/core/src/guardian/review_session/mod.rs

@@ -30,7 +30,6 @@ use crate::thread_manager::snapshot_rollout_history;
 
 use self::execution::run_before_review_deadline;
 use self::execution::run_review_on_session;
-use self::spawn::GuardianReviewSessionReuseKey;
 use self::spawn::GuardianReviewSessionSpawnOutcome;
 #[cfg(test)]
 pub(crate) use self::spawn::build_guardian_review_session_config;
@@ -141,8 +140,10 @@ struct GuardianReviewSession {
     codex: Codex,
     /// Session-scoped cancellation used during shutdown.
     cancel_token: CancellationToken,
-    /// Spawn-config fingerprint used to decide when the cached trunk is still reusable.
-    reuse_key: GuardianReviewSessionReuseKey,
+    /// Effective guardian config used for this child session.
+    ///
+    /// The trunk remains reusable only while future reviews resolve to the same config.
+    spawn_config: Config,
     /// Tracks whether this session has already completed at least one review turn.
     has_prior_review: AtomicBool,
     /// Prevents overlapping reviews on the same guardian session.
@@ -153,13 +154,13 @@ impl GuardianReviewSession {
     fn new(
         codex: Codex,
         cancel_token: CancellationToken,
-        reuse_key: GuardianReviewSessionReuseKey,
+        spawn_config: Config,
         has_prior_review: bool,
     ) -> Self {
         Self {
             codex,
             cancel_token,
-            reuse_key,
+            spawn_config,
             has_prior_review: AtomicBool::new(has_prior_review),
             review_lock: Mutex::new(()),
         }
@@ -318,14 +319,8 @@ impl GuardianReviewSessionManager {
         params: GuardianReviewSessionParams,
     ) -> GuardianReviewSessionOutcome {
         let deadline = tokio::time::Instant::now() + GUARDIAN_REVIEW_TIMEOUT;
-        let next_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(&params.spawn_config);
         let trunk = match self
-            .get_or_spawn_trunk_for_review(
-                &params,
-                &next_reuse_key,
-                deadline,
-                params.external_cancel.as_ref(),
-            )
+            .get_or_spawn_trunk_for_review(&params, deadline, params.external_cancel.as_ref())
             .await
         {
             Ok(Some(trunk)) => trunk,
@@ -335,14 +330,9 @@ impl GuardianReviewSessionManager {
 
         // A stale-but-busy trunk stays in place so the in-flight review can finish. New work forks
         // instead of replacing the live session.
-        if trunk.reuse_key != next_reuse_key {
+        if trunk.spawn_config != params.spawn_config {
             return self
-                .run_forked_review(
-                    params,
-                    next_reuse_key,
-                    deadline,
-                    /*initial_history*/ None,
-                )
+                .run_forked_review(params, deadline, /*initial_history*/ None)
                 .await;
         }
 
@@ -351,7 +341,7 @@ impl GuardianReviewSessionManager {
             Err(_) => {
                 let initial_history = trunk.fork_initial_history().await;
                 return self
-                    .run_forked_review(params, next_reuse_key, deadline, initial_history)
+                    .run_forked_review(params, deadline, initial_history)
                     .await;
             }
         };
@@ -383,7 +373,6 @@ impl GuardianReviewSessionManager {
         spawn_config: Config,
         eager_init_cancel: &CancellationToken,
     ) {
-        let next_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(&spawn_config);
         let Ok(_spawn_guard) = self.spawn_lock.try_lock() else {
             return;
         };
@@ -399,7 +388,6 @@ impl GuardianReviewSessionManager {
                 parent_session,
                 parent_turn,
                 spawn_config,
-                next_reuse_key,
             )
             .await
         {
@@ -424,11 +412,10 @@ impl GuardianReviewSessionManager {
     async fn get_or_spawn_trunk_for_review(
         &self,
         params: &GuardianReviewSessionParams,
-        next_reuse_key: &GuardianReviewSessionReuseKey,
         deadline: tokio::time::Instant,
         external_cancel: Option<&CancellationToken>,
     ) -> Result<Option<Arc<GuardianReviewSession>>, GuardianReviewSessionOutcome> {
-        match self.prepare_trunk(next_reuse_key).await {
+        match self.prepare_trunk(&params.spawn_config).await {
             GuardianTrunkState::Ready(trunk) => return Ok(Some(trunk)),
             GuardianTrunkState::ShutdownStarted => return Ok(None),
             GuardianTrunkState::NeedsSpawn => {}
@@ -443,7 +430,7 @@ impl GuardianReviewSessionManager {
             };
 
         // Another task may have finished spawning while we were waiting on `spawn_lock`.
-        let trunk = match self.prepare_trunk(next_reuse_key).await {
+        let trunk = match self.prepare_trunk(&params.spawn_config).await {
             GuardianTrunkState::Ready(trunk) => Some(trunk),
             GuardianTrunkState::ShutdownStarted => None,
             GuardianTrunkState::NeedsSpawn => {
@@ -454,7 +441,6 @@ impl GuardianReviewSessionManager {
                         &params.parent_session,
                         &params.parent_turn,
                         params.spawn_config.clone(),
-                        next_reuse_key.clone(),
                     )
                     .await
                 {
@@ -477,7 +463,6 @@ impl GuardianReviewSessionManager {
         parent_session: &Arc<Session>,
         parent_turn: &Arc<TurnContext>,
         spawn_config: Config,
-        next_reuse_key: GuardianReviewSessionReuseKey,
     ) -> Result<Option<Arc<GuardianReviewSession>>, GuardianReviewSessionSpawnOutcome> {
         // Spawn under the caller's deadline policy first, then register the result against shared
         // trunk state exactly once in `install_spawned_trunk`.
@@ -487,7 +472,6 @@ impl GuardianReviewSessionManager {
             parent_session,
             parent_turn,
             spawn_config,
-            next_reuse_key,
             /*initial_history*/ None,
         )
         .await?;
@@ -496,17 +480,14 @@ impl GuardianReviewSessionManager {
 
     /// Inspects the cached trunk and eagerly evicts a stale idle trunk so the caller can spawn a
     /// replacement. Busy trunks are left in place.
-    async fn prepare_trunk(
-        &self,
-        next_reuse_key: &GuardianReviewSessionReuseKey,
-    ) -> GuardianTrunkState {
+    async fn prepare_trunk(&self, next_spawn_config: &Config) -> GuardianTrunkState {
         let (trunk_state, stale_trunk_to_shutdown) = {
             let mut state = self.state.lock().await;
             if state.shutdown_started {
                 return GuardianTrunkState::ShutdownStarted;
             }
             if let Some(trunk) = state.trunk.as_ref()
-                && trunk.reuse_key != *next_reuse_key
+                && trunk.spawn_config != *next_spawn_config
                 && trunk.review_lock.try_lock().is_ok()
             {
                 (GuardianTrunkState::NeedsSpawn, state.trunk.take())
@@ -591,22 +572,18 @@ impl GuardianReviewSessionManager {
 
     #[cfg(test)]
     pub(crate) async fn cache_for_test(&self, codex: Codex) {
-        let reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(
-            codex.session.get_config().await.as_ref(),
-        );
+        let spawn_config = codex.session.get_config().await.as_ref().clone();
         self.state.lock().await.trunk = Some(Arc::new(GuardianReviewSession::new(
             codex,
             CancellationToken::new(),
-            reuse_key,
+            spawn_config,
             /*has_prior_review*/ false,
         )));
     }
 
     #[cfg(test)]
     pub(crate) async fn register_fork_for_test(&self, codex: Codex) {
-        let reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(
-            codex.session.get_config().await.as_ref(),
-        );
+        let spawn_config = codex.session.get_config().await.as_ref().clone();
         self.state
             .lock()
             .await
@@ -614,7 +591,7 @@ impl GuardianReviewSessionManager {
             .push(Arc::new(GuardianReviewSession::new(
                 codex,
                 CancellationToken::new(),
-                reuse_key,
+                spawn_config,
                 /*has_prior_review*/ false,
             )));
     }
@@ -622,7 +599,6 @@ impl GuardianReviewSessionManager {
     async fn run_forked_review(
         &self,
         params: GuardianReviewSessionParams,
-        reuse_key: GuardianReviewSessionReuseKey,
         deadline: tokio::time::Instant,
         initial_history: Option<InitialHistory>,
     ) -> GuardianReviewSessionOutcome {
@@ -636,7 +612,6 @@ impl GuardianReviewSessionManager {
             &params.parent_session,
             &params.parent_turn,
             fork_config,
-            reuse_key,
             initial_history,
         )
         .await

codex-rs/core/src/guardian/review_session/spawn.rs

@@ -5,8 +5,6 @@
 //! - what config/model should the guardian child session use?
 //! - how do we spawn that child session under the caller's deadline/cancel policy?
 
-use std::collections::HashMap;
-use std::path::PathBuf;
 use std::sync::Arc;
 
 use codex_features::Feature;
@@ -21,11 +19,7 @@ use crate::codex::TurnContext;
 use crate::codex_delegate::run_codex_thread_interactive;
 use crate::config::Config;
 use crate::config::Constrained;
-use crate::config::ManagedFeatures;
 use crate::config::NetworkProxySpec;
-use crate::config::Permissions;
-use crate::config::types::McpServerConfig;
-use crate::model_provider_info::ModelProviderInfo;
 use crate::models_manager::manager::RefreshStrategy;
 use crate::protocol::SandboxPolicy;
 
@@ -50,62 +44,6 @@ pub(super) enum GuardianReviewSessionSpawnOutcome {
     Aborted,
 }
 
-#[derive(Debug, Clone, PartialEq)]
-pub(super) struct GuardianReviewSessionReuseKey {
-    // Only include settings that affect spawned-session behavior so reuse
-    // invalidation remains explicit and does not depend on unrelated config
-    // bookkeeping.
-    model: Option<String>,
-    model_provider_id: String,
-    model_provider: ModelProviderInfo,
-    model_context_window: Option<i64>,
-    model_auto_compact_token_limit: Option<i64>,
-    model_reasoning_effort: Option<ReasoningEffortConfig>,
-    permissions: Permissions,
-    developer_instructions: Option<String>,
-    base_instructions: Option<String>,
-    user_instructions: Option<String>,
-    compact_prompt: Option<String>,
-    cwd: PathBuf,
-    mcp_servers: Constrained<HashMap<String, McpServerConfig>>,
-    codex_linux_sandbox_exe: Option<PathBuf>,
-    main_execve_wrapper_exe: Option<PathBuf>,
-    js_repl_node_path: Option<PathBuf>,
-    js_repl_node_module_dirs: Vec<PathBuf>,
-    zsh_path: Option<PathBuf>,
-    features: ManagedFeatures,
-    include_apply_patch_tool: bool,
-    use_experimental_unified_exec_tool: bool,
-}
-
-impl GuardianReviewSessionReuseKey {
-    pub(super) fn from_spawn_config(spawn_config: &Config) -> Self {
-        Self {
-            model: spawn_config.model.clone(),
-            model_provider_id: spawn_config.model_provider_id.clone(),
-            model_provider: spawn_config.model_provider.clone(),
-            model_context_window: spawn_config.model_context_window,
-            model_auto_compact_token_limit: spawn_config.model_auto_compact_token_limit,
-            model_reasoning_effort: spawn_config.model_reasoning_effort,
-            permissions: spawn_config.permissions.clone(),
-            developer_instructions: spawn_config.developer_instructions.clone(),
-            base_instructions: spawn_config.base_instructions.clone(),
-            user_instructions: spawn_config.user_instructions.clone(),
-            compact_prompt: spawn_config.compact_prompt.clone(),
-            cwd: spawn_config.cwd.clone(),
-            mcp_servers: spawn_config.mcp_servers.clone(),
-            codex_linux_sandbox_exe: spawn_config.codex_linux_sandbox_exe.clone(),
-            main_execve_wrapper_exe: spawn_config.main_execve_wrapper_exe.clone(),
-            js_repl_node_path: spawn_config.js_repl_node_path.clone(),
-            js_repl_node_module_dirs: spawn_config.js_repl_node_module_dirs.clone(),
-            zsh_path: spawn_config.zsh_path.clone(),
-            features: spawn_config.features.clone(),
-            include_apply_patch_tool: spawn_config.include_apply_patch_tool,
-            use_experimental_unified_exec_tool: spawn_config.use_experimental_unified_exec_tool,
-        }
-    }
-}
-
 /// Spawns a guardian child session and maps deadline/cancel outcomes into a small internal enum.
 ///
 /// Trunk creation and fork creation both use this helper so they do not duplicate the same
@@ -116,7 +54,6 @@ pub(super) async fn spawn_review_session_before_deadline(
     parent_session: &Arc<Session>,
     parent_turn: &Arc<TurnContext>,
     spawn_config: Config,
-    reuse_key: GuardianReviewSessionReuseKey,
     initial_history: Option<InitialHistory>,
 ) -> Result<Arc<GuardianReviewSession>, GuardianReviewSessionSpawnOutcome> {
     let spawn_cancel_token = tokio_util::sync::CancellationToken::new();
@@ -128,7 +65,6 @@ pub(super) async fn spawn_review_session_before_deadline(
             Arc::clone(parent_session),
             Arc::clone(parent_turn),
             spawn_config,
-            reuse_key,
             spawn_cancel_token.clone(),
             initial_history,
         )),
@@ -168,11 +104,11 @@ async fn spawn_guardian_review_session(
     parent_session: Arc<Session>,
     parent_turn: Arc<TurnContext>,
     spawn_config: Config,
-    reuse_key: GuardianReviewSessionReuseKey,
     cancel_token: tokio_util::sync::CancellationToken,
     initial_history: Option<InitialHistory>,
 ) -> anyhow::Result<GuardianReviewSession> {
     let has_prior_review = initial_history.is_some();
+    let review_config = spawn_config.clone();
     // Guardian runs as an ordinary child Codex thread with a different config and source label.
     let codex = run_codex_thread_interactive(
         spawn_config,
@@ -189,7 +125,7 @@ async fn spawn_guardian_review_session(
     Ok(GuardianReviewSession::new(
         codex,
         cancel_token,
-        reuse_key,
+        review_config,
         has_prior_review,
     ))
 }

codex-rs/core/src/guardian/review_session/tests.rs

@@ -24,7 +24,6 @@ async fn guardian_review_session_with_shutdown_signal() -> (
     let (child_session, _child_turn_context) = crate::codex::make_session_and_context().await;
     let child_session = Arc::new(child_session);
     let child_config = child_session.get_config().await;
-    let reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(child_config.as_ref());
     let (child_tx_sub, child_rx_sub) = async_channel::bounded(4);
     let (_child_tx_event, child_rx_event) = async_channel::unbounded();
     let (_child_status_tx, child_agent_status) = watch::channel(AgentStatus::PendingInit);
@@ -49,7 +48,7 @@ async fn guardian_review_session_with_shutdown_signal() -> (
     let review_session = Arc::new(GuardianReviewSession::new(
         child_codex,
         CancellationToken::new(),
-        reuse_key,
+        child_config.as_ref().clone(),
         /*has_prior_review*/ false,
     ));
 
@@ -57,12 +56,11 @@ async fn guardian_review_session_with_shutdown_signal() -> (
 }
 
 #[test]
-fn guardian_review_session_config_change_invalidates_cached_session() {
+fn guardian_review_session_config_change_changes_spawn_config() {
     let parent_config = crate::config::test_config();
     let cached_spawn_config =
         build_guardian_review_session_config(&parent_config, None, "active-model", None)
             .expect("cached guardian config");
-    let cached_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(&cached_spawn_config);
 
     let mut changed_parent_config = parent_config;
     changed_parent_config.model_provider.base_url =
@@ -70,13 +68,8 @@ fn guardian_review_session_config_change_invalidates_cached_session() {
     let next_spawn_config =
         build_guardian_review_session_config(&changed_parent_config, None, "active-model", None)
             .expect("next guardian config");
-    let next_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(&next_spawn_config);
 
-    assert_ne!(cached_reuse_key, next_reuse_key);
-    assert_eq!(
-        cached_reuse_key,
-        GuardianReviewSessionReuseKey::from_spawn_config(&cached_spawn_config)
-    );
+    assert_ne!(cached_spawn_config, next_spawn_config);
 }
 
 #[test]