From cab607befb2f613854961c0988b0821879bc27f5 Mon Sep 17 00:00:00 2001 From: Charley Cunningham Date: Tue, 17 Feb 2026 09:21:44 -0800 Subject: [PATCH] Centralize context update diffing logic (#11807) ## Summary This PR centralizes model-visible state diffing for turn context updates into one module, while keeping existing behavior and call sites stable. ### What changed - Added `core/src/context_updates.rs` with the consolidated diffing logic for: - environment context updates - permissions/policy updates - collaboration mode updates - model-instruction switch updates - personality updates - Added `BuildSettingsUpdateItemsParams` so required dependencies are passed explicitly. - Updated `Session::build_settings_update_items` in `core/src/codex.rs` to delegate to the centralized module. - Reused the same centralized `personality_message_for` helper from initial-context assembly to avoid duplicated logic. - Registered the new module in `core/src/lib.rs`. ## Why This is a minimal, shippable step toward the model-visible-state design: all state diff decisions for turn-context update items now live in one place, improving reviewability and reducing drift risk without expanding scope. ## Behavior - Intended to be behavior-preserving. - No protocol/schema changes. - No call-site behavior changes beyond routing through the new centralized logic. ## Testing Ran targeted tests in this worktree: - `cargo test -p codex-core build_settings_update_items_emits_environment_item_for_network_changes` - `cargo test -p codex-core collaboration_instructions --test all` Both passed. ## Codex author `codex resume 019c540f-3951-7352-a3fa-6f07b834d4ce` --- codex-rs/core/src/codex.rs | 158 ++----------------- codex-rs/core/src/context_manager/mod.rs | 1 + codex-rs/core/src/context_manager/updates.rs | 148 +++++++++++++++++ 3 files changed, 165 insertions(+), 142 deletions(-) create mode 100644 codex-rs/core/src/context_manager/updates.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2b0a76809..34f0d9f87 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1969,119 +1969,6 @@ impl Session { state.session_configuration.collaboration_mode.clone() } - fn build_environment_update_item( - &self, - previous: Option<&Arc>, - next: &TurnContext, - ) -> Option { - let prev = previous?; - - let shell = self.user_shell(); - let prev_context = EnvironmentContext::from_turn_context(prev.as_ref(), shell.as_ref()); - let next_context = EnvironmentContext::from_turn_context(next, shell.as_ref()); - if prev_context.equals_except_shell(&next_context) { - return None; - } - Some(ResponseItem::from(EnvironmentContext::diff( - prev.as_ref(), - next, - shell.as_ref(), - ))) - } - - fn build_permissions_update_item( - &self, - previous: Option<&Arc>, - next: &TurnContext, - ) -> Option { - let prev = previous?; - if prev.sandbox_policy == next.sandbox_policy - && prev.approval_policy == next.approval_policy - { - return None; - } - - Some( - DeveloperInstructions::from_policy( - &next.sandbox_policy, - next.approval_policy, - self.services.exec_policy.current().as_ref(), - &next.cwd, - ) - .into(), - ) - } - - fn build_personality_update_item( - &self, - previous: Option<&Arc>, - next: &TurnContext, - ) -> Option { - if !self.features.enabled(Feature::Personality) { - return None; - } - let previous = previous?; - if next.model_info.slug != previous.model_info.slug { - return None; - } - - // if a personality is specified and it's different from the previous one, build a personality update item - if let Some(personality) = next.personality - && next.personality != previous.personality - { - let model_info = &next.model_info; - let personality_message = Self::personality_message_for(model_info, personality); - personality_message.map(|personality_message| { - DeveloperInstructions::personality_spec_message(personality_message).into() - }) - } else { - None - } - } - - fn personality_message_for(model_info: &ModelInfo, personality: Personality) -> Option { - model_info - .model_messages - .as_ref() - .and_then(|spec| spec.get_personality_message(Some(personality))) - .filter(|message| !message.is_empty()) - } - - fn build_collaboration_mode_update_item( - &self, - previous: Option<&Arc>, - next: &TurnContext, - ) -> Option { - let prev = previous?; - if prev.collaboration_mode != next.collaboration_mode { - // If the next mode has empty developer instructions, this returns None and we emit no - // update, so prior collaboration instructions remain in the prompt history. - Some(DeveloperInstructions::from_collaboration_mode(&next.collaboration_mode)?.into()) - } else { - None - } - } - - fn build_model_instructions_update_item( - &self, - previous: Option<&Arc>, - resumed_model: Option<&str>, - next: &TurnContext, - ) -> Option { - let previous_model = - resumed_model.or_else(|| previous.map(|prev| prev.model_info.slug.as_str()))?; - if previous_model == next.model_info.slug { - return None; - } - - let model_instructions = next.model_info.get_model_instructions(next.personality); - if model_instructions.is_empty() { - return None; - } - - Some(DeveloperInstructions::model_switch_message(model_instructions).into()) - } - pub(crate) fn is_model_switch_developer_message(item: &ResponseItem) -> bool { let ResponseItem::Message { role, content, .. } = item else { return false; @@ -2094,42 +1981,26 @@ impl Session { ) }) } - fn build_settings_update_items( &self, previous_context: Option<&Arc>, resumed_model: Option<&str>, current_context: &TurnContext, ) -> Vec { - let mut update_items = Vec::new(); - if let Some(env_item) = - self.build_environment_update_item(previous_context, current_context) - { - update_items.push(env_item); - } - if let Some(permissions_item) = - self.build_permissions_update_item(previous_context, current_context) - { - update_items.push(permissions_item); - } - if let Some(collaboration_mode_item) = - self.build_collaboration_mode_update_item(previous_context, current_context) - { - update_items.push(collaboration_mode_item); - } - if let Some(model_instructions_item) = self.build_model_instructions_update_item( - previous_context, + // TODO: Make context updates a pure diff of persisted previous/current TurnContextItem + // state so replay/backtracking is deterministic. Runtime inputs that affect model-visible + // context (shell, exec policy, feature gates, resumed model bridge) should be persisted + // state or explicit non-state replay events. + let shell = self.user_shell(); + let exec_policy = self.services.exec_policy.current(); + crate::context_manager::updates::build_settings_update_items( + previous_context.map(Arc::as_ref), resumed_model, current_context, - ) { - update_items.push(model_instructions_item); - } - if let Some(personality_item) = - self.build_personality_update_item(previous_context, current_context) - { - update_items.push(personality_item); - } - update_items + shell.as_ref(), + exec_policy.as_ref(), + self.features.enabled(Feature::Personality), + ) } /// Persist the event to rollout and send it to clients. @@ -2689,7 +2560,10 @@ impl Session { && base_instructions == model_info.get_model_instructions(Some(personality)); if !has_baked_personality && let Some(personality_message) = - Self::personality_message_for(&model_info, personality) + crate::context_manager::updates::personality_message_for( + &model_info, + personality, + ) { items.push( DeveloperInstructions::personality_spec_message(personality_message).into(), diff --git a/codex-rs/core/src/context_manager/mod.rs b/codex-rs/core/src/context_manager/mod.rs index dcaf794c4..853f8af5a 100644 --- a/codex-rs/core/src/context_manager/mod.rs +++ b/codex-rs/core/src/context_manager/mod.rs @@ -1,5 +1,6 @@ mod history; mod normalize; +pub(crate) mod updates; pub(crate) use history::ContextManager; pub(crate) use history::TotalTokenUsageBreakdown; diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs new file mode 100644 index 000000000..4a3f0c15e --- /dev/null +++ b/codex-rs/core/src/context_manager/updates.rs @@ -0,0 +1,148 @@ +use crate::codex::TurnContext; +use crate::environment_context::EnvironmentContext; +use crate::shell::Shell; +use codex_execpolicy::Policy; +use codex_protocol::config_types::Personality; +use codex_protocol::models::DeveloperInstructions; +use codex_protocol::models::ResponseItem; +use codex_protocol::openai_models::ModelInfo; + +fn build_environment_update_item( + previous: Option<&TurnContext>, + next: &TurnContext, + shell: &Shell, +) -> Option { + let prev = previous?; + let prev_context = EnvironmentContext::from_turn_context(prev, shell); + let next_context = EnvironmentContext::from_turn_context(next, shell); + if prev_context.equals_except_shell(&next_context) { + return None; + } + + Some(ResponseItem::from(EnvironmentContext::diff( + prev, next, shell, + ))) +} + +fn build_permissions_update_item( + previous: Option<&TurnContext>, + next: &TurnContext, + exec_policy: &Policy, +) -> Option { + let prev = previous?; + if prev.sandbox_policy == next.sandbox_policy && prev.approval_policy == next.approval_policy { + return None; + } + + Some( + DeveloperInstructions::from_policy( + &next.sandbox_policy, + next.approval_policy, + exec_policy, + &next.cwd, + ) + .into(), + ) +} + +fn build_collaboration_mode_update_item( + previous: Option<&TurnContext>, + next: &TurnContext, +) -> Option { + let prev = previous?; + if prev.collaboration_mode != next.collaboration_mode { + // If the next mode has empty developer instructions, this returns None and we emit no + // update, so prior collaboration instructions remain in the prompt history. + Some(DeveloperInstructions::from_collaboration_mode(&next.collaboration_mode)?.into()) + } else { + None + } +} + +fn build_personality_update_item( + previous: Option<&TurnContext>, + next: &TurnContext, + personality_feature_enabled: bool, +) -> Option { + if !personality_feature_enabled { + return None; + } + let previous = previous?; + if next.model_info.slug != previous.model_info.slug { + return None; + } + + if let Some(personality) = next.personality + && next.personality != previous.personality + { + let model_info = &next.model_info; + let personality_message = personality_message_for(model_info, personality); + personality_message + .map(|message| DeveloperInstructions::personality_spec_message(message).into()) + } else { + None + } +} + +pub(crate) fn personality_message_for( + model_info: &ModelInfo, + personality: Personality, +) -> Option { + model_info + .model_messages + .as_ref() + .and_then(|spec| spec.get_personality_message(Some(personality))) + .filter(|message| !message.is_empty()) +} + +pub(crate) fn build_model_instructions_update_item( + previous: Option<&TurnContext>, + resumed_model: Option<&str>, + next: &TurnContext, +) -> Option { + let previous_model = + resumed_model.or_else(|| previous.map(|prev| prev.model_info.slug.as_str()))?; + if previous_model == next.model_info.slug { + return None; + } + + let model_instructions = next.model_info.get_model_instructions(next.personality); + if model_instructions.is_empty() { + return None; + } + + Some(DeveloperInstructions::model_switch_message(model_instructions).into()) +} + +pub(crate) fn build_settings_update_items( + previous: Option<&TurnContext>, + resumed_model: Option<&str>, + next: &TurnContext, + shell: &Shell, + exec_policy: &Policy, + personality_feature_enabled: bool, +) -> Vec { + let mut update_items = Vec::new(); + + if let Some(env_item) = build_environment_update_item(previous, next, shell) { + update_items.push(env_item); + } + if let Some(permissions_item) = build_permissions_update_item(previous, next, exec_policy) { + update_items.push(permissions_item); + } + if let Some(collaboration_mode_item) = build_collaboration_mode_update_item(previous, next) { + update_items.push(collaboration_mode_item); + } + if let Some(model_instructions_item) = + build_model_instructions_update_item(previous, resumed_model, next) + { + update_items.push(model_instructions_item); + } + if let Some(personality_item) = + build_personality_update_item(previous, next, personality_feature_enabled) + { + update_items.push(personality_item); + } + + update_items +}