diff --git a/AGENTS.md b/AGENTS.md index 85d61141ae..b7ccef1392 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,13 +41,14 @@ In the codex-rs folder where the rust code lives: - Model-visible prompt context should go through the shared fragment abstractions described in `docs/model-visible-context.md`. - Every new model-visible fragment should implement `ModelVisibleContextFragment` and set `type Role`. - Turn-state model-visible context assembly should produce exactly two envelopes (one developer message + one contextual-user message) via the shared envelope builders. -- Contextual-user fragments should use the shared detection path in `model_visible_context`: implement `ContextualUserFragment`, prefer defining `markers()` when marker-based detection/wrapping is sufficient, and override `matches_contextual_user_text()` only for genuinely custom matching (for example AGENTS.md). +- Define and register current model-visible fragment types in `codex-rs/core/src/model_visible_fragments.rs`. That registry is the single source of truth for contextual-user detection and both turn-state envelopes. +- Contextual-user fragments should use the shared detection path on `ModelVisibleContextFragment`: prefer defining `contextual_user_markers()` when marker-based detection/wrapping is sufficient, and override `matches_contextual_user_text()` only for genuinely custom matching (for example AGENTS.md). - Use the developer envelope for developer guidance. Custom override text (for example config/app-server `developer_instructions`) should use `CustomDeveloperInstructions`; system-generated developer context should use typed fragments plus the neutral `developer_*_text` helpers rather than reusing the custom override type. - Use the contextual-user envelope for user-role contextual state or runtime markers such as AGENTS instructions, plugin instructions, environment context, skills, and shell-command markers. Contextual-user fragments must provide stable markers so history parsing treats them as contextual state rather than user intent. - Use `` specifically for environment facts derived from `TurnContext` that may need turn-to-turn diffs (`cwd`, `shell`, optional `current_date`, optional `timezone`, optional network allow/deny domain summaries). Do not put policy text, plugin/skill listings, or other guidance into ``; those should use dedicated fragments. -- Fragments derived from durable/current turn state that should update/reinject via diff across resume/fork/compaction/backtracking should implement `TurnContextDiffFragment` so current-state extraction, diffing, and rendering live together. -- Runtime/session-prefix one-off fragments can implement only `ModelVisibleContextFragment` when they are not turn-state diffs. -- Register new turn-state fragments by type in the single ordered registry used by both initial-context and turn-diff assembly (`REGISTERED_TURN_STATE_FRAGMENT_BUILDERS` in `context_manager/updates.rs`) via `build_registered_turn_state_fragment::`. +- Fragments derived from durable/current turn state that should update/reinject via diff across resume/fork/compaction/backtracking should implement `ModelVisibleContextFragment::build(...)` so current-state extraction, diffing, and rendering live together. +- Runtime/session-prefix one-off fragments should leave `ModelVisibleContextFragment::build(...)` as `None` when they are not turn-state diffs. +- Register every current fragment exactly once in `REGISTERED_MODEL_VISIBLE_FRAGMENTS` in `codex-rs/core/src/model_visible_fragments.rs`, in the rough order it should appear in model-visible context. - Do not hand-construct model-visible `ResponseItem::Message` payloads in new code; use fragment conversion and shared envelope builders. - Do not inject raw strings directly into the initial-context or settings-update builders, and do not call fragment wrapping helpers ad hoc from new code. diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index a8540ab706..8b50ed201c 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -6,9 +6,9 @@ use crate::agent::status::is_final; use crate::error::CodexErr; use crate::error::Result as CodexResult; use crate::find_thread_path_by_id_str; +use crate::model_visible_fragments::SubagentNotification; +use crate::model_visible_fragments::format_subagent_context_line; use crate::rollout::RolloutRecorder; -use crate::session_prefix::SubagentNotification; -use crate::session_prefix::format_subagent_context_line; use crate::shell_snapshot::ShellSnapshot; use crate::state_db; use crate::thread_manager::ThreadManagerState; diff --git a/codex-rs/core/src/context_manager/updates/developer_update_fragments.rs b/codex-rs/core/src/context_manager/updates/developer_update_fragments.rs deleted file mode 100644 index 08a3369832..0000000000 --- a/codex-rs/core/src/context_manager/updates/developer_update_fragments.rs +++ /dev/null @@ -1,299 +0,0 @@ -//! Developer-envelope model-visible fragments used by turn-state context -//! assembly. -//! -//! This module owns the turn-context diffing logic for developer-role context -//! updates (permissions, collaboration mode, realtime, personality, and model -//! switch guidance). - -use crate::codex::TurnContext; -use crate::features::Feature; -use crate::model_visible_context::DeveloperContextRole; -use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::TurnContextDiffFragment; -use crate::model_visible_context::TurnContextDiffParams; -use codex_protocol::models::developer_collaboration_mode_text; -use codex_protocol::models::developer_model_switch_text; -use codex_protocol::models::developer_permissions_text; -use codex_protocol::models::developer_personality_spec_text; -use codex_protocol::models::developer_realtime_end_text; -use codex_protocol::models::developer_realtime_start_text_with_instructions; -use codex_protocol::protocol::TurnContextItem; - -// --------------------------------------------------------------------------- -// Model instructions fragment -// --------------------------------------------------------------------------- - -pub(super) struct ModelInstructionsUpdateFragment { - text: String, -} - -impl ModelVisibleContextFragment for ModelInstructionsUpdateFragment { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - self.text.clone() - } -} - -impl TurnContextDiffFragment for ModelInstructionsUpdateFragment { - fn build( - turn_context: &TurnContext, - _reference_context_item: Option<&TurnContextItem>, - params: &TurnContextDiffParams<'_>, - ) -> Option { - let previous_model = params - .previous_turn_settings - .map(|settings| settings.model.as_str()); - let previous_model = previous_model?; - if previous_model == turn_context.model_info.slug.as_str() { - return None; - } - - let model_instructions = turn_context - .model_info - .get_model_instructions(turn_context.personality); - if model_instructions.is_empty() { - return None; - } - - Some(Self { - text: developer_model_switch_text(model_instructions), - }) - } -} - -// --------------------------------------------------------------------------- -// Permissions fragment -// --------------------------------------------------------------------------- - -pub(super) struct PermissionsUpdateFragment { - text: String, -} - -impl ModelVisibleContextFragment for PermissionsUpdateFragment { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - self.text.clone() - } -} - -impl TurnContextDiffFragment for PermissionsUpdateFragment { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - params: &TurnContextDiffParams<'_>, - ) -> Option { - if reference_context_item.is_some_and(|previous| { - previous.sandbox_policy == *turn_context.sandbox_policy.get() - && previous.approval_policy == turn_context.approval_policy.value() - }) { - return None; - } - - Some(Self { - text: developer_permissions_text( - turn_context.sandbox_policy.get(), - turn_context.approval_policy.value(), - turn_context.features.enabled(Feature::GuardianApproval), - params.exec_policy, - &turn_context.cwd, - turn_context - .features - .enabled(Feature::ExecPermissionApprovals) - || turn_context - .features - .enabled(Feature::RequestPermissionsTool), - ), - }) - } -} - -// --------------------------------------------------------------------------- -// Custom developer instructions fragment -// --------------------------------------------------------------------------- - -pub(super) struct CustomDeveloperInstructionsUpdateFragment { - text: String, -} - -impl ModelVisibleContextFragment for CustomDeveloperInstructionsUpdateFragment { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - self.text.clone() - } -} - -impl TurnContextDiffFragment for CustomDeveloperInstructionsUpdateFragment { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - _params: &TurnContextDiffParams<'_>, - ) -> Option { - if reference_context_item.is_some_and(|previous| { - previous.developer_instructions == turn_context.developer_instructions - }) { - return None; - } - - Some(Self { - text: turn_context.developer_instructions.as_ref()?.clone(), - }) - } -} - -// --------------------------------------------------------------------------- -// Collaboration mode fragment -// --------------------------------------------------------------------------- - -pub(super) struct CollaborationModeUpdateFragment { - text: String, -} - -impl ModelVisibleContextFragment for CollaborationModeUpdateFragment { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - self.text.clone() - } -} - -impl TurnContextDiffFragment for CollaborationModeUpdateFragment { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - _params: &TurnContextDiffParams<'_>, - ) -> Option { - if let Some(previous) = reference_context_item { - if previous.collaboration_mode.as_ref() != Some(&turn_context.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. - return Some(Self { - text: developer_collaboration_mode_text(&turn_context.collaboration_mode)?, - }); - } - return None; - } - - developer_collaboration_mode_text(&turn_context.collaboration_mode) - .map(|text| Self { text }) - } -} - -// --------------------------------------------------------------------------- -// Realtime fragment -// --------------------------------------------------------------------------- - -pub(super) struct RealtimeUpdateFragment { - text: String, -} - -impl ModelVisibleContextFragment for RealtimeUpdateFragment { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - self.text.clone() - } -} - -impl TurnContextDiffFragment for RealtimeUpdateFragment { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - params: &TurnContextDiffParams<'_>, - ) -> Option { - let text = match ( - reference_context_item.and_then(|previous| previous.realtime_active), - turn_context.realtime_active, - ) { - (Some(true), false) => Some(developer_realtime_end_text("inactive")), - (Some(false), true) | (None, true) => { - Some(developer_realtime_start_text_with_instructions( - turn_context - .config - .experimental_realtime_start_instructions - .as_deref(), - )) - } - (Some(true), true) | (Some(false), false) => None, - (None, false) => params - .previous_turn_settings - .and_then(|settings| settings.realtime_active) - .filter(|realtime_active| *realtime_active) - .map(|_| developer_realtime_end_text("inactive")), - }?; - - Some(Self { text }) - } -} - -// --------------------------------------------------------------------------- -// Personality fragment -// --------------------------------------------------------------------------- - -pub(super) struct PersonalityUpdateFragment { - text: String, -} - -impl ModelVisibleContextFragment for PersonalityUpdateFragment { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - self.text.clone() - } -} - -impl TurnContextDiffFragment for PersonalityUpdateFragment { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - params: &TurnContextDiffParams<'_>, - ) -> Option { - if !params.personality_feature_enabled { - return None; - } - - let Some(previous) = reference_context_item else { - let personality = turn_context.personality?; - let has_baked_personality = params.base_instructions.is_some_and(|base_instructions| { - turn_context.model_info.supports_personality() - && base_instructions - == turn_context - .model_info - .get_model_instructions(Some(personality)) - }); - if has_baked_personality { - return None; - } - let personality_message = turn_context - .model_info - .model_messages - .as_ref() - .and_then(|spec| spec.get_personality_message(Some(personality))) - .filter(|message| !message.is_empty())?; - return Some(Self { - text: developer_personality_spec_text(personality_message), - }); - }; - - if turn_context.model_info.slug != previous.model { - return None; - } - if let Some(personality) = turn_context.personality - && turn_context.personality != previous.personality - { - let personality_message = turn_context - .model_info - .model_messages - .as_ref() - .and_then(|spec| spec.get_personality_message(Some(personality))) - .filter(|message| !message.is_empty())?; - return Some(Self { - text: developer_personality_spec_text(personality_message), - }); - } - - None - } -} diff --git a/codex-rs/core/src/contextual_user_message_tests.rs b/codex-rs/core/src/contextual_user_message_tests.rs deleted file mode 100644 index df3a9daeca..0000000000 --- a/codex-rs/core/src/contextual_user_message_tests.rs +++ /dev/null @@ -1,31 +0,0 @@ -use super::*; - -#[test] -fn detects_environment_context_fragment() { - assert!(is_contextual_user_fragment(&ContentItem::InputText { - text: "\n/tmp\n".to_string(), - })); -} - -#[test] -fn detects_agents_instructions_fragment() { - assert!(is_contextual_user_fragment(&ContentItem::InputText { - text: "# AGENTS.md instructions for /tmp\n\n\nbody\n" - .to_string(), - })); -} - -#[test] -fn detects_subagent_notification_fragment_case_insensitively() { - assert!( - SUBAGENT_NOTIFICATION_FRAGMENT - .matches_text("{}") - ); -} - -#[test] -fn ignores_regular_user_text() { - assert!(!is_contextual_user_fragment(&ContentItem::InputText { - text: "hello".to_string(), - })); -} diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/environment_context.rs deleted file mode 100644 index d959959f0f..0000000000 --- a/codex-rs/core/src/environment_context.rs +++ /dev/null @@ -1,198 +0,0 @@ -use crate::codex::TurnContext; -use crate::model_visible_context::ContextualUserContextRole; -use crate::model_visible_context::ContextualUserFragment; -use crate::model_visible_context::ContextualUserFragmentMarkers; -use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::TurnContextDiffFragment; -use crate::model_visible_context::TurnContextDiffParams; -use crate::shell::Shell; -use codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG; -use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; -use codex_protocol::protocol::TurnContextItem; -use codex_protocol::protocol::TurnContextNetworkItem; -use serde::Deserialize; -use serde::Serialize; -use std::path::PathBuf; - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(rename = "environment_context", rename_all = "snake_case")] -pub(crate) struct EnvironmentContext { - pub cwd: Option, - pub shell: Shell, - pub current_date: Option, - pub timezone: Option, - pub network: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] -pub(crate) struct NetworkContext { - allowed_domains: Vec, - denied_domains: Vec, -} - -impl EnvironmentContext { - const MARKERS: ContextualUserFragmentMarkers = ContextualUserFragmentMarkers::new( - ENVIRONMENT_CONTEXT_OPEN_TAG, - ENVIRONMENT_CONTEXT_CLOSE_TAG, - ); - - pub fn new( - cwd: Option, - shell: Shell, - current_date: Option, - timezone: Option, - network: Option, - ) -> Self { - Self { - cwd, - shell, - current_date, - timezone, - network, - } - } - - /// Compares two environment contexts, ignoring the shell. Useful when - /// comparing turn to turn, since the initial environment_context will - /// include the shell, and then it is not configurable from turn to turn. - pub fn equals_except_shell(&self, other: &EnvironmentContext) -> bool { - let EnvironmentContext { - cwd, - current_date, - timezone, - network, - shell: _, - } = other; - self.cwd == *cwd - && self.current_date == *current_date - && self.timezone == *timezone - && self.network == *network - } - - fn network_from_turn_context(turn_context: &TurnContext) -> Option { - let network = turn_context - .config - .config_layer_stack - .requirements() - .network - .as_ref()?; - - Some(NetworkContext { - allowed_domains: network.allowed_domains.clone().unwrap_or_default(), - denied_domains: network.denied_domains.clone().unwrap_or_default(), - }) - } - - fn network_from_turn_context_item( - turn_context_item: &TurnContextItem, - ) -> Option { - let TurnContextNetworkItem { - allowed_domains, - denied_domains, - } = turn_context_item.network.as_ref()?; - Some(NetworkContext { - allowed_domains: allowed_domains.clone(), - denied_domains: denied_domains.clone(), - }) - } -} - -impl ModelVisibleContextFragment for EnvironmentContext { - type Role = ContextualUserContextRole; - - fn render_text(&self) -> String { - let mut lines = Vec::new(); - if let Some(cwd) = &self.cwd { - lines.push(format!(" {}", cwd.to_string_lossy())); - } - - let shell_name = self.shell.name(); - lines.push(format!(" {shell_name}")); - if let Some(current_date) = &self.current_date { - lines.push(format!(" {current_date}")); - } - if let Some(timezone) = &self.timezone { - lines.push(format!(" {timezone}")); - } - match &self.network { - Some(network) => { - lines.push(" ".to_string()); - for allowed in &network.allowed_domains { - lines.push(format!(" {allowed}")); - } - for denied in &network.denied_domains { - lines.push(format!(" {denied}")); - } - lines.push(" ".to_string()); - } - None => { - // TODO(mbolin): Include this line if it helps the model. - // lines.push(" ".to_string()); - } - } - Self::MARKERS.wrap_body(lines.join("\n")) - } -} - -impl ContextualUserFragment for EnvironmentContext { - fn markers() -> Option { - Some(Self::MARKERS) - } -} - -impl TurnContextDiffFragment for EnvironmentContext { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - params: &TurnContextDiffParams<'_>, - ) -> Option { - let current_network = Self::network_from_turn_context(turn_context); - let current_context = Self::new( - Some(turn_context.cwd.clone()), - params.shell.clone(), - turn_context.current_date.clone(), - turn_context.timezone.clone(), - current_network.clone(), - ); - - let Some(previous) = reference_context_item else { - return Some(current_context); - }; - - let previous_network = Self::network_from_turn_context_item(previous); - let previous_context = Self::new( - Some(previous.cwd.clone()), - params.shell.clone(), - previous.current_date.clone(), - previous.timezone.clone(), - previous_network.clone(), - ); - - if previous_context.equals_except_shell(¤t_context) { - return None; - } - - let cwd = if previous.cwd != turn_context.cwd { - Some(turn_context.cwd.clone()) - } else { - None - }; - let network = if previous_network != current_network { - current_network - } else { - previous_network - }; - - Some(Self::new( - cwd, - params.shell.clone(), - turn_context.current_date.clone(), - turn_context.timezone.clone(), - network, - )) - } -} - -#[cfg(test)] -#[path = "environment_context_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 51e244330d..1c4a7cf5c1 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -18,7 +18,7 @@ use codex_protocol::user_input::UserInput; use tracing::warn; use uuid::Uuid; -use crate::model_visible_context::is_contextual_user_fragment; +use crate::model_visible_fragments::is_contextual_user_fragment; use crate::web_search::web_search_action_detail; pub(crate) fn is_contextual_user_message_content(message: &[ContentItem]) -> bool { diff --git a/codex-rs/core/src/instructions/contextual_user_fragments.rs b/codex-rs/core/src/instructions/contextual_user_fragments.rs deleted file mode 100644 index ae9922e9ae..0000000000 --- a/codex-rs/core/src/instructions/contextual_user_fragments.rs +++ /dev/null @@ -1,255 +0,0 @@ -//! Contextual-user model-visible fragments used by initial-context assembly. -//! -//! These fragments represent injected user-role context (for example AGENTS.md, -//! skills, and plugin guidance) and include turn-context extraction/diffing for -//! AGENTS.md instructions. - -use serde::Deserialize; -use serde::Serialize; - -use crate::codex::TurnContext; -use crate::model_visible_context::ContextualUserContextRole; -use crate::model_visible_context::ContextualUserFragment; -use crate::model_visible_context::ContextualUserFragmentMarkers; -use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::PLUGINS_CLOSE_TAG; -use crate::model_visible_context::PLUGINS_OPEN_TAG; -use crate::model_visible_context::SKILL_CLOSE_TAG; -use crate::model_visible_context::SKILL_OPEN_TAG; -use crate::model_visible_context::TurnContextDiffFragment; -use crate::model_visible_context::TurnContextDiffParams; -use codex_protocol::protocol::TurnContextItem; - -// --------------------------------------------------------------------------- -// AGENTS instructions fragment -// --------------------------------------------------------------------------- - -const AGENTS_MD_START_MARKER: &str = "# AGENTS.md instructions for "; -const AGENTS_MD_END_MARKER: &str = ""; - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(rename = "user_instructions", rename_all = "snake_case")] -pub(crate) struct AgentsMdInstructions { - pub directory: String, - pub text: String, -} - -impl ModelVisibleContextFragment for AgentsMdInstructions { - type Role = ContextualUserContextRole; - - fn render_text(&self) -> String { - // TODO(ccunningham): Switch AGENTS.md rendering/detection to - // `...` for consistency with the - // other contextual-user fragments. - format!( - "{prefix}{directory}\n\n\n{contents}\n{suffix}", - prefix = AGENTS_MD_START_MARKER, - directory = self.directory, - contents = self.text, - suffix = AGENTS_MD_END_MARKER, - ) - } -} - -impl TurnContextDiffFragment for AgentsMdInstructions { - fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - _params: &TurnContextDiffParams<'_>, - ) -> Option { - let text = turn_context.user_instructions.as_ref()?.clone(); - let current = Self { - directory: turn_context.cwd.to_string_lossy().into_owned(), - text, - }; - if let Some(previous) = reference_context_item { - let previous_directory = previous.cwd.to_string_lossy().into_owned(); - if previous_directory == current.directory - && previous.user_instructions.as_deref() == Some(current.text.as_str()) - { - return None; - } - } - - Some(current) - } -} - -impl ContextualUserFragment for AgentsMdInstructions { - fn matches_contextual_user_text(text: &str) -> bool { - let trimmed = text.trim_start(); - // TODO(ccunningham): Switch detection to the XML-ish wrapper once we - // intentionally change the shipped AGENTS.md fragment format. - trimmed.starts_with(AGENTS_MD_START_MARKER) - && trimmed.trim_end().ends_with(AGENTS_MD_END_MARKER) - } -} - -// --------------------------------------------------------------------------- -// Skills fragment -// --------------------------------------------------------------------------- - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(rename = "skill_instructions", rename_all = "snake_case")] -pub(crate) struct SkillInstructions { - pub name: String, - pub path: String, - pub contents: String, -} - -impl ModelVisibleContextFragment for SkillInstructions { - type Role = ContextualUserContextRole; - - fn render_text(&self) -> String { - ::wrap_contextual_user_body(format!( - "{}\n{}\n{}", - self.name, self.path, self.contents - )) - } -} - -impl ContextualUserFragment for SkillInstructions { - fn markers() -> Option { - Some(ContextualUserFragmentMarkers::new( - SKILL_OPEN_TAG, - SKILL_CLOSE_TAG, - )) - } -} - -// --------------------------------------------------------------------------- -// Plugins fragment -// --------------------------------------------------------------------------- - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(rename = "plugin_instructions", rename_all = "snake_case")] -pub(crate) struct PluginInstructions { - pub text: String, -} - -impl ModelVisibleContextFragment for PluginInstructions { - type Role = ContextualUserContextRole; - - fn render_text(&self) -> String { - ::wrap_contextual_user_body(self.text.clone()) - } -} - -impl ContextualUserFragment for PluginInstructions { - fn markers() -> Option { - Some(ContextualUserFragmentMarkers::new( - PLUGINS_OPEN_TAG, - PLUGINS_CLOSE_TAG, - )) - } -} - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -#[cfg(test)] -mod tests { - use super::*; - use codex_protocol::models::ContentItem; - use codex_protocol::models::ResponseItem; - use pretty_assertions::assert_eq; - - #[test] - fn test_user_instructions() { - let user_instructions = AgentsMdInstructions { - directory: "test_directory".to_string(), - text: "test_text".to_string(), - }; - let response_item = user_instructions.into_message(); - - let ResponseItem::Message { role, content, .. } = response_item else { - panic!("expected ResponseItem::Message"); - }; - - assert_eq!(role, "user"); - - let [ContentItem::InputText { text }] = content.as_slice() else { - panic!("expected one InputText content item"); - }; - - assert_eq!( - text, - "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", - ); - } - - #[test] - fn test_is_user_instructions() { - assert!(crate::model_visible_context::is_contextual_user_fragment( - &ContentItem::InputText { - text: "# AGENTS.md instructions for test_directory\n\n\ntest_text\n" - .to_string(), - } - )); - assert!( - ::matches_contextual_user_text( - "# AGENTS.md instructions for test_directory\n\n\ntest_text\n" - ) - ); - } - - #[test] - fn test_skill_instructions() { - let skill_instructions = SkillInstructions { - name: "demo-skill".to_string(), - path: "skills/demo/SKILL.md".to_string(), - contents: "body".to_string(), - }; - let response_item = skill_instructions.into_message(); - - let ResponseItem::Message { role, content, .. } = response_item else { - panic!("expected ResponseItem::Message"); - }; - - assert_eq!(role, "user"); - - let [ContentItem::InputText { text }] = content.as_slice() else { - panic!("expected one InputText content item"); - }; - - assert_eq!( - text, - "\ndemo-skill\nskills/demo/SKILL.md\nbody\n", - ); - } - - #[test] - fn test_is_skill_instructions() { - assert!( - ::matches_contextual_user_text( - "\ndemo-skill\nskills/demo/SKILL.md\nbody\n" - ) - ); - assert!( - !::matches_contextual_user_text( - "regular text" - ) - ); - } - - #[test] - fn test_plugin_instructions() { - let plugin_instructions = PluginInstructions { - text: "## Plugins\n- `sample`".to_string(), - }; - let response_item = plugin_instructions.into_message(); - - let ResponseItem::Message { role, content, .. } = response_item else { - panic!("expected ResponseItem::Message"); - }; - - assert_eq!(role, "user"); - - let [ContentItem::InputText { text }] = content.as_slice() else { - panic!("expected one InputText content item"); - }; - - assert_eq!(text, "\n## Plugins\n- `sample`\n"); - } -} diff --git a/codex-rs/core/src/instructions/mod.rs b/codex-rs/core/src/instructions/mod.rs deleted file mode 100644 index 8c22f67e3b..0000000000 --- a/codex-rs/core/src/instructions/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -mod contextual_user_fragments; - -pub(crate) use contextual_user_fragments::AgentsMdInstructions; -pub(crate) use contextual_user_fragments::PluginInstructions; -pub(crate) use contextual_user_fragments::SkillInstructions; diff --git a/codex-rs/core/src/instructions/user_instructions_tests.rs b/codex-rs/core/src/instructions/user_instructions_tests.rs deleted file mode 100644 index 58442600a8..0000000000 --- a/codex-rs/core/src/instructions/user_instructions_tests.rs +++ /dev/null @@ -1,68 +0,0 @@ -use super::*; -use codex_protocol::models::ContentItem; -use pretty_assertions::assert_eq; - -#[test] -fn test_user_instructions() { - let user_instructions = UserInstructions { - directory: "test_directory".to_string(), - text: "test_text".to_string(), - }; - let response_item: ResponseItem = user_instructions.into(); - - let ResponseItem::Message { role, content, .. } = response_item else { - panic!("expected ResponseItem::Message"); - }; - - assert_eq!(role, "user"); - - let [ContentItem::InputText { text }] = content.as_slice() else { - panic!("expected one InputText content item"); - }; - - assert_eq!( - text, - "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", - ); -} - -#[test] -fn test_is_user_instructions() { - assert!(AGENTS_MD_FRAGMENT.matches_text( - "# AGENTS.md instructions for test_directory\n\n\ntest_text\n" - )); - assert!(!AGENTS_MD_FRAGMENT.matches_text("test_text")); -} - -#[test] -fn test_skill_instructions() { - let skill_instructions = SkillInstructions { - name: "demo-skill".to_string(), - path: "skills/demo/SKILL.md".to_string(), - contents: "body".to_string(), - }; - let response_item: ResponseItem = skill_instructions.into(); - - let ResponseItem::Message { role, content, .. } = response_item else { - panic!("expected ResponseItem::Message"); - }; - - assert_eq!(role, "user"); - - let [ContentItem::InputText { text }] = content.as_slice() else { - panic!("expected one InputText content item"); - }; - - assert_eq!( - text, - "\ndemo-skill\nskills/demo/SKILL.md\nbody\n", - ); -} - -#[test] -fn test_is_skill_instructions() { - assert!(SKILL_FRAGMENT.matches_text( - "\ndemo-skill\nskills/demo/SKILL.md\nbody\n" - )); - assert!(!SKILL_FRAGMENT.matches_text("regular text")); -} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 48cfbaddf6..9025d0b2cd 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -31,7 +31,6 @@ pub mod connectors; mod context_manager; pub mod custom_prompts; pub mod env; -mod environment_context; pub mod error; pub mod exec; pub mod exec_env; @@ -42,12 +41,12 @@ mod file_watcher; mod flags; pub mod git_info; mod guardian; -pub mod instructions; pub mod landlock; pub mod mcp; mod mcp_connection_manager; mod mcp_tool_approval_templates; mod model_visible_context; +mod model_visible_fragments; pub mod models_manager; mod network_policy_decision; pub mod network_proxy_loader; @@ -67,7 +66,6 @@ pub mod personality_migration; pub mod plugins; mod sandbox_tags; pub mod sandboxing; -mod session_prefix; mod shell_detect; mod stream_events_utils; pub mod test_support; @@ -145,7 +143,6 @@ pub use rollout::session_index::find_thread_names_by_ids; mod function_tool; mod state; mod tasks; -mod user_shell_command; pub mod util; pub(crate) use codex_protocol::protocol; pub(crate) use codex_shell_command::bash; diff --git a/codex-rs/core/src/model_visible_context.rs b/codex-rs/core/src/model_visible_context.rs index e96e078d32..fa31ef1a20 100644 --- a/codex-rs/core/src/model_visible_context.rs +++ b/codex-rs/core/src/model_visible_context.rs @@ -3,19 +3,13 @@ //! Use this path for any injected prompt context, whether it renders in the //! developer envelope or the contextual-user envelope. //! -//! Contextual-user fragments must provide stable markers so history parsing can -//! distinguish them from real user intent. Developer fragments do not need -//! markers because they are already separable by role. +//! Fragment registration and concrete fragment definitions live in +//! `model_visible_fragments.rs`. This module keeps only the shared rendering, +//! role, and turn-context parameter helpers that every fragment uses. use crate::codex::PreviousTurnSettings; use crate::codex::TurnContext; -use crate::environment_context::EnvironmentContext; -use crate::instructions::AgentsMdInstructions; -use crate::instructions::PluginInstructions; -use crate::instructions::SkillInstructions; use crate::shell::Shell; -use crate::tasks::TurnAbortedMarker; -use crate::user_shell_command::UserShellCommandFragment; use codex_execpolicy::Policy; use codex_protocol::models::ContentItem; use codex_protocol::models::CustomDeveloperInstructions; @@ -59,23 +53,6 @@ pub(crate) struct ContextualUserFragmentMarkers { end_marker: &'static str, } -pub(crate) trait ContextualUserFragment { - fn markers() -> Option { - None - } - - fn matches_contextual_user_text(text: &str) -> bool { - Self::markers().is_some_and(|markers| markers.matches_text(text)) - } - - fn wrap_contextual_user_body(body: String) -> String { - let Some(markers) = Self::markers() else { - panic!("contextual-user fragments using wrap_contextual_user_body must define markers"); - }; - markers.wrap_body(body) - } -} - impl ContextualUserFragmentMarkers { pub(crate) const fn new(start_marker: &'static str, end_marker: &'static str) -> Self { Self { @@ -124,79 +101,6 @@ pub(crate) fn model_visible_response_input_item( } } -/// Implement this for any model-visible prompt fragment, regardless of which -/// envelope it renders into. -pub(crate) trait ModelVisibleContextFragment { - type Role: ModelVisibleContextRole; - - fn render_text(&self) -> String; - - fn into_content_item(self) -> ContentItem - where - Self: Sized, - { - model_visible_content_item(self.render_text()) - } - - fn into_message(self) -> ResponseItem - where - Self: Sized, - { - model_visible_message::(self.render_text()) - } - - fn into_response_input_item(self) -> ResponseInputItem - where - Self: Sized, - { - model_visible_response_input_item::(self.render_text()) - } -} - -pub(crate) struct DeveloperTextFragment { - text: String, -} - -impl DeveloperTextFragment { - pub(crate) fn new(text: impl Into) -> Self { - Self { text: text.into() } - } -} - -pub(crate) struct ContextualUserTextFragment { - text: String, -} - -impl ContextualUserTextFragment { - pub(crate) fn new(text: impl Into) -> Self { - Self { text: text.into() } - } -} - -type ContextualUserTurnStateBuilder = fn( - Option<&TurnContextItem>, - &TurnContext, - &TurnContextDiffParams<'_>, -) -> Option; - -#[derive(Clone, Copy)] -struct ContextualUserFragmentRegistration { - detect: fn(&str) -> bool, - turn_state_builder: Option, -} - -impl ContextualUserFragmentRegistration { - const fn new( - detect: fn(&str) -> bool, - turn_state_builder: Option, - ) -> Self { - Self { - detect, - turn_state_builder, - } - } -} - pub(crate) struct TurnContextDiffParams<'a> { pub(crate) shell: &'a Shell, pub(crate) previous_turn_settings: Option<&'a PreviousTurnSettings>, @@ -223,123 +127,75 @@ impl<'a> TurnContextDiffParams<'a> { } } -/// Implement this for fragments that are built from current/persisted turn -/// state rather than one-off runtime events. -pub(crate) trait TurnContextDiffFragment: ModelVisibleContextFragment + Sized { +/// Implement this for any model-visible prompt fragment, regardless of which +/// envelope it renders into. +pub(crate) trait ModelVisibleContextFragment: Sized { + type Role: ModelVisibleContextRole; + + fn render_text(&self) -> String; + /// Build the fragment from the current turn state and an optional baseline - /// context item. + /// context item that represents the turn state already reflected in + /// model-visible history. /// - /// `reference_context_item` is the last persisted turn-context snapshot whose - /// effects are already represented in model-visible history. Implementations - /// should diff `turn_context` against this baseline and return `None` when - /// there is no model-visible change to inject. - /// - /// `reference_context_item` is `None` for initial-context assembly and when - /// no baseline turn context can be recovered (for example after - /// compaction/backtracking/resume), so implementations should treat that as - /// "no known represented baseline" and decide whether to emit full current - /// state or nothing. + /// Implementations that are not turn-state fragments should leave the + /// default `None`. fn build( - turn_context: &TurnContext, - reference_context_item: Option<&TurnContextItem>, - params: &TurnContextDiffParams<'_>, - ) -> Option; + _turn_context: &TurnContext, + _reference_context_item: Option<&TurnContextItem>, + _params: &TurnContextDiffParams<'_>, + ) -> Option { + None + } + + /// Stable markers used to recognize contextual-user fragments in persisted + /// history. Developer fragments should keep the default `None`. + fn contextual_user_markers() -> Option { + None + } + + fn matches_contextual_user_text(text: &str) -> bool { + Self::contextual_user_markers().is_some_and(|markers| markers.matches_text(text)) + } + + fn wrap_contextual_user_body(body: String) -> String { + let Some(markers) = Self::contextual_user_markers() else { + panic!("contextual-user fragments using wrap_contextual_user_body must define markers"); + }; + markers.wrap_body(body) + } + + fn into_content_item(self) -> ContentItem { + model_visible_content_item(self.render_text()) + } + + fn into_message(self) -> ResponseItem { + model_visible_message::(self.render_text()) + } + + fn into_response_input_item(self) -> ResponseInputItem { + model_visible_response_input_item::(self.render_text()) + } } -fn detect_contextual_user_fragment(text: &str) -> bool { - F::matches_contextual_user_text(text) +pub(crate) struct DeveloperTextFragment { + text: String, } -fn build_contextual_user_turn_state_fragment( - reference_context_item: Option<&TurnContextItem>, - turn_context: &TurnContext, - params: &TurnContextDiffParams<'_>, -) -> Option -where - F: TurnContextDiffFragment + ContextualUserFragment, -{ - let fragment = F::build(turn_context, reference_context_item, params)?; - Some(ContextualUserTextFragment::new(fragment.render_text())) +impl DeveloperTextFragment { + pub(crate) fn new(text: impl Into) -> Self { + Self { text: text.into() } + } } -/// Canonical contextual-user fragment registry. -/// -/// Add new contextual-user fragments by: -/// 1. Defining a typed fragment struct. -/// 2. Implementing `ModelVisibleContextFragment` with -/// `Role = ContextualUserContextRole`. -/// 3. Implementing `ContextualUserFragment` for detection. Prefer defining -/// `markers()` so the default matcher/wrapper behavior applies; override -/// `matches_contextual_user_text()` only for genuinely custom formats. -/// 4. If the fragment is derived from turn state, implementing -/// `TurnContextDiffFragment::build` and registering it with -/// `Some(build_contextual_user_turn_state_fragment::)`. -/// 5. Otherwise registering it with `None` for diffing so it still participates -/// in contextual-user history parsing. -/// -/// Register new fragment types here so injected context is not mistaken for -/// real user intent during event mapping/truncation, and to wire turn-state -/// contextual-user diff fragments in one place. -const REGISTERED_CONTEXTUAL_USER_FRAGMENTS: &[ContextualUserFragmentRegistration] = &[ - ContextualUserFragmentRegistration::new( - detect_contextual_user_fragment::, - Some(build_contextual_user_turn_state_fragment::), - ), - ContextualUserFragmentRegistration::new( - detect_contextual_user_fragment::, - Some(build_contextual_user_turn_state_fragment::), - ), - ContextualUserFragmentRegistration::new( - detect_contextual_user_fragment::, - None, - ), - ContextualUserFragmentRegistration::new( - detect_contextual_user_fragment::, - None, - ), - ContextualUserFragmentRegistration::new( - detect_contextual_user_fragment::, - None, - ), - ContextualUserFragmentRegistration::new( - detect_contextual_user_fragment::, - None, - ), -]; - -fn is_legacy_contextual_user_fragment(text: &str) -> bool { - // TODO(ccunningham): Drop this once old user-role subagent notification - // history no longer needs resume/compaction compatibility. - ContextualUserFragmentMarkers::new( - SUBAGENT_NOTIFICATION_OPEN_TAG, - SUBAGENT_NOTIFICATION_CLOSE_TAG, - ) - .matches_text(text) +pub(crate) struct ContextualUserTextFragment { + text: String, } -pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool { - let ContentItem::InputText { text } = content_item else { - return false; - }; - REGISTERED_CONTEXTUAL_USER_FRAGMENTS - .iter() - .any(|registration| (registration.detect)(text)) - || is_legacy_contextual_user_fragment(text) -} - -pub(crate) fn build_contextual_user_turn_state_fragments( - reference_context_item: Option<&TurnContextItem>, - turn_context: &TurnContext, - params: &TurnContextDiffParams<'_>, -) -> Vec { - REGISTERED_CONTEXTUAL_USER_FRAGMENTS - .iter() - .filter_map(|registration| { - registration - .turn_state_builder - .and_then(|build| build(reference_context_item, turn_context, params)) - }) - .collect() +impl ContextualUserTextFragment { + pub(crate) fn new(text: impl Into) -> Self { + Self { text: text.into() } + } } impl ModelVisibleContextFragment for CustomDeveloperInstructions { @@ -365,45 +221,3 @@ impl ModelVisibleContextFragment for ContextualUserTextFragment { self.text.clone() } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn detects_environment_context_fragment() { - assert!(is_contextual_user_fragment(&ContentItem::InputText { - text: "\n/tmp\n".to_string(), - })); - } - - #[test] - fn detects_agents_instructions_fragment() { - assert!(is_contextual_user_fragment(&ContentItem::InputText { - text: "# AGENTS.md instructions for /tmp\n\n\nbody\n" - .to_string(), - })); - } - - #[test] - fn detects_legacy_subagent_notification_fragment() { - assert!(is_contextual_user_fragment(&ContentItem::InputText { - text: "\n{\"agent_id\":\"a\",\"status\":\"completed\"}\n" - .to_string(), - })); - } - - #[test] - fn ignores_regular_user_text() { - assert!(!is_contextual_user_fragment(&ContentItem::InputText { - text: "hello".to_string(), - })); - } - - #[test] - fn marker_matching_ignores_plain_text() { - assert!( - !::matches_contextual_user_text("plain text") - ); - } -} diff --git a/codex-rs/core/src/model_visible_fragments.rs b/codex-rs/core/src/model_visible_fragments.rs new file mode 100644 index 0000000000..666a5db8dd --- /dev/null +++ b/codex-rs/core/src/model_visible_fragments.rs @@ -0,0 +1,940 @@ +//! Canonical model-visible fragment definitions and registration. +//! +//! This is the single place to add new model-visible prompt context. +//! +//! Turn-state context is always assembled into exactly two envelopes: +//! - one developer message +//! - one contextual-user message +//! +//! Add a new fragment by: +//! 1. Defining a typed fragment struct in this file. +//! 2. Implementing `ModelVisibleContextFragment`, including `type Role`. +//! 3. If the fragment is contextual-user state, defining +//! `contextual_user_markers()` or overriding +//! `matches_contextual_user_text()` for custom matching. +//! 4. If the fragment is derived from `TurnContext` and should participate in +//! initial-context assembly and turn-to-turn diffing, implementing +//! `build(...)`. +//! 5. Registering the fragment exactly once in +//! `REGISTERED_MODEL_VISIBLE_FRAGMENTS` in the rough order it should appear +//! in model-visible context. +//! +//! The registry drives: +//! - contextual-user history detection +//! - turn-state fragment assembly for both envelopes +//! +//! Fragments that are only emitted as runtime/session-prefix messages should +//! leave `build(...)` as `None`; they still belong here so detection and +//! rendering stay standardized. + +use crate::codex::TurnContext; +use crate::exec::ExecToolCallOutput; +use crate::features::Feature; +use crate::model_visible_context::ContextualUserContextRole; +use crate::model_visible_context::ContextualUserFragmentMarkers; +use crate::model_visible_context::ContextualUserTextFragment; +use crate::model_visible_context::DeveloperContextRole; +use crate::model_visible_context::DeveloperTextFragment; +use crate::model_visible_context::ModelVisibleContextFragment; +use crate::model_visible_context::ModelVisibleContextRole; +use crate::model_visible_context::PLUGINS_CLOSE_TAG; +use crate::model_visible_context::PLUGINS_OPEN_TAG; +use crate::model_visible_context::SKILL_CLOSE_TAG; +use crate::model_visible_context::SKILL_OPEN_TAG; +use crate::model_visible_context::SUBAGENT_NOTIFICATION_CLOSE_TAG; +use crate::model_visible_context::SUBAGENT_NOTIFICATION_OPEN_TAG; +use crate::model_visible_context::SUBAGENTS_CLOSE_TAG; +use crate::model_visible_context::SUBAGENTS_OPEN_TAG; +use crate::model_visible_context::TURN_ABORTED_CLOSE_TAG; +use crate::model_visible_context::TURN_ABORTED_OPEN_TAG; +use crate::model_visible_context::TurnContextDiffParams; +use crate::model_visible_context::USER_SHELL_COMMAND_CLOSE_TAG; +use crate::model_visible_context::USER_SHELL_COMMAND_OPEN_TAG; +use crate::shell::Shell; +use crate::tools::format_exec_output_str; +use codex_protocol::models::ContentItem; +use codex_protocol::models::MessageRole; +use codex_protocol::models::ResponseItem; +use codex_protocol::models::developer_collaboration_mode_text; +use codex_protocol::models::developer_model_switch_text; +use codex_protocol::models::developer_permissions_text; +use codex_protocol::models::developer_personality_spec_text; +use codex_protocol::models::developer_realtime_end_text; +use codex_protocol::models::developer_realtime_start_text_with_instructions; +use codex_protocol::protocol::AgentStatus; +use codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG; +use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; +use codex_protocol::protocol::TurnContextItem; +use codex_protocol::protocol::TurnContextNetworkItem; +use serde::Deserialize; +use serde::Serialize; +use std::path::PathBuf; +use std::time::Duration; + +pub(crate) enum BuiltTurnStateFragment { + Developer(DeveloperTextFragment), + ContextualUser(ContextualUserTextFragment), +} + +#[derive(Clone, Copy)] +struct ModelVisibleFragmentRegistration { + detect_contextual_user: fn(&str) -> bool, + build_turn_state: fn( + Option<&TurnContextItem>, + &TurnContext, + &TurnContextDiffParams<'_>, + ) -> Option, +} + +impl ModelVisibleFragmentRegistration { + const fn of() -> Self { + Self { + detect_contextual_user: detect_registered_contextual_user_fragment::, + build_turn_state: build_registered_turn_state_fragment::, + } + } +} + +fn detect_registered_contextual_user_fragment(text: &str) -> bool { + if F::Role::MESSAGE_ROLE != MessageRole::User { + return false; + } + F::matches_contextual_user_text(text) +} + +fn build_registered_turn_state_fragment( + reference_context_item: Option<&TurnContextItem>, + turn_context: &TurnContext, + params: &TurnContextDiffParams<'_>, +) -> Option { + let fragment = F::build(turn_context, reference_context_item, params)?; + match F::Role::MESSAGE_ROLE { + MessageRole::Developer => Some(BuiltTurnStateFragment::Developer( + DeveloperTextFragment::new(fragment.render_text()), + )), + MessageRole::User => Some(BuiltTurnStateFragment::ContextualUser( + ContextualUserTextFragment::new(fragment.render_text()), + )), + MessageRole::Assistant | MessageRole::System => None, + } +} + +/// Canonical ordered registry for all current model-visible fragments. +const REGISTERED_MODEL_VISIBLE_FRAGMENTS: &[ModelVisibleFragmentRegistration] = &[ + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), + ModelVisibleFragmentRegistration::of::(), +]; + +// --------------------------------------------------------------------------- +// Developer-envelope turn-state fragments +// --------------------------------------------------------------------------- + +pub(crate) struct ModelInstructionsUpdateFragment { + text: String, +} + +impl ModelVisibleContextFragment for ModelInstructionsUpdateFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn build( + turn_context: &TurnContext, + _reference_context_item: Option<&TurnContextItem>, + params: &TurnContextDiffParams<'_>, + ) -> Option { + let previous_model = params + .previous_turn_settings + .map(|settings| settings.model.as_str())?; + if previous_model == turn_context.model_info.slug.as_str() { + return None; + } + + let model_instructions = turn_context + .model_info + .get_model_instructions(turn_context.personality); + if model_instructions.is_empty() { + return None; + } + + Some(Self { + text: developer_model_switch_text(model_instructions), + }) + } +} + +pub(crate) struct PermissionsUpdateFragment { + text: String, +} + +impl ModelVisibleContextFragment for PermissionsUpdateFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + params: &TurnContextDiffParams<'_>, + ) -> Option { + if reference_context_item.is_some_and(|previous| { + previous.sandbox_policy == *turn_context.sandbox_policy.get() + && previous.approval_policy == turn_context.approval_policy.value() + }) { + return None; + } + + Some(Self { + text: developer_permissions_text( + turn_context.sandbox_policy.get(), + turn_context.approval_policy.value(), + turn_context.features.enabled(Feature::GuardianApproval), + params.exec_policy, + &turn_context.cwd, + turn_context + .features + .enabled(Feature::ExecPermissionApprovals) + || turn_context + .features + .enabled(Feature::RequestPermissionsTool), + ), + }) + } +} + +pub(crate) struct CustomDeveloperInstructionsUpdateFragment { + text: String, +} + +impl ModelVisibleContextFragment for CustomDeveloperInstructionsUpdateFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + _params: &TurnContextDiffParams<'_>, + ) -> Option { + if reference_context_item.is_some_and(|previous| { + previous.developer_instructions == turn_context.developer_instructions + }) { + return None; + } + + Some(Self { + text: turn_context.developer_instructions.as_ref()?.clone(), + }) + } +} + +pub(crate) struct CollaborationModeUpdateFragment { + text: String, +} + +impl ModelVisibleContextFragment for CollaborationModeUpdateFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + _params: &TurnContextDiffParams<'_>, + ) -> Option { + if let Some(previous) = reference_context_item { + let previous_mode = previous.collaboration_mode.as_ref()?; + let previous_text = developer_collaboration_mode_text(previous_mode); + let current_text = developer_collaboration_mode_text(&turn_context.collaboration_mode); + if previous_text == current_text { + return None; + } + + let text = current_text.unwrap_or_else(|| { + format!( + "# Collaboration Mode: {}\n\nYou are now in {} mode. Any previous instructions for other modes are no longer active.", + turn_context.collaboration_mode.mode.display_name(), + turn_context.collaboration_mode.mode.display_name(), + ) + }); + return Some(Self { text }); + } + + developer_collaboration_mode_text(&turn_context.collaboration_mode) + .map(|text| Self { text }) + } +} + +pub(crate) struct RealtimeUpdateFragment { + text: String, +} + +impl ModelVisibleContextFragment for RealtimeUpdateFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + params: &TurnContextDiffParams<'_>, + ) -> Option { + let text = match ( + reference_context_item.and_then(|previous| previous.realtime_active), + turn_context.realtime_active, + ) { + (Some(true), false) => Some(developer_realtime_end_text("inactive")), + (Some(false), true) | (None, true) => { + Some(developer_realtime_start_text_with_instructions( + turn_context + .config + .experimental_realtime_start_instructions + .as_deref(), + )) + } + (Some(true), true) | (Some(false), false) => None, + (None, false) => params + .previous_turn_settings + .and_then(|settings| settings.realtime_active) + .filter(|realtime_active| *realtime_active) + .map(|_| developer_realtime_end_text("inactive")), + }?; + + Some(Self { text }) + } +} + +pub(crate) struct PersonalityUpdateFragment { + text: String, +} + +impl ModelVisibleContextFragment for PersonalityUpdateFragment { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + params: &TurnContextDiffParams<'_>, + ) -> Option { + if !params.personality_feature_enabled { + return None; + } + + let Some(previous) = reference_context_item else { + let personality = turn_context.personality?; + let has_baked_personality = params.base_instructions.is_some_and(|base_instructions| { + turn_context.model_info.supports_personality() + && base_instructions + == turn_context + .model_info + .get_model_instructions(Some(personality)) + }); + if has_baked_personality { + return None; + } + + let personality_message = turn_context + .model_info + .model_messages + .as_ref() + .and_then(|spec| spec.get_personality_message(Some(personality))) + .filter(|message| !message.is_empty())?; + return Some(Self { + text: developer_personality_spec_text(personality_message), + }); + }; + + if turn_context.model_info.slug != previous.model { + return None; + } + if let Some(personality) = turn_context.personality + && turn_context.personality != previous.personality + { + let personality_message = turn_context + .model_info + .model_messages + .as_ref() + .and_then(|spec| spec.get_personality_message(Some(personality))) + .filter(|message| !message.is_empty())?; + return Some(Self { + text: developer_personality_spec_text(personality_message), + }); + } + + None + } +} + +// --------------------------------------------------------------------------- +// Developer runtime fragments +// --------------------------------------------------------------------------- + +pub(crate) struct SubagentRosterContext { + subagents: String, +} + +impl SubagentRosterContext { + pub(crate) fn new(subagents: String) -> Option { + if subagents.is_empty() { + None + } else { + Some(Self { subagents }) + } + } +} + +impl ModelVisibleContextFragment for SubagentRosterContext { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + let lines = self + .subagents + .lines() + .map(|line| format!(" {line}")) + .collect::>() + .join("\n"); + format!("{SUBAGENTS_OPEN_TAG}\n{lines}\n{SUBAGENTS_CLOSE_TAG}") + } +} + +pub(crate) struct SubagentNotification { + agent_id: String, + status: AgentStatus, +} + +impl SubagentNotification { + pub(crate) fn new(agent_id: &str, status: &AgentStatus) -> Self { + Self { + agent_id: agent_id.to_string(), + status: status.clone(), + } + } +} + +impl ModelVisibleContextFragment for SubagentNotification { + type Role = DeveloperContextRole; + + fn render_text(&self) -> String { + let payload_json = serde_json::json!({ + "agent_id": self.agent_id, + "status": self.status, + }) + .to_string(); + format!( + "{SUBAGENT_NOTIFICATION_OPEN_TAG}\n{payload_json}\n{SUBAGENT_NOTIFICATION_CLOSE_TAG}" + ) + } +} + +pub(crate) fn format_subagent_context_line(agent_id: &str, agent_nickname: Option<&str>) -> String { + match agent_nickname.filter(|nickname| !nickname.is_empty()) { + Some(agent_nickname) => format!("- {agent_id}: {agent_nickname}"), + None => format!("- {agent_id}"), + } +} + +// --------------------------------------------------------------------------- +// Contextual-user turn-state fragments +// --------------------------------------------------------------------------- + +const AGENTS_MD_START_MARKER: &str = "# AGENTS.md instructions for "; +const AGENTS_MD_END_MARKER: &str = ""; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename = "user_instructions", rename_all = "snake_case")] +pub(crate) struct AgentsMdInstructions { + pub directory: String, + pub text: String, +} + +impl ModelVisibleContextFragment for AgentsMdInstructions { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + // TODO(ccunningham): Switch AGENTS.md rendering/detection to + // `...` for consistency with the + // other contextual-user fragments. + format!( + "{AGENTS_MD_START_MARKER}{directory}\n\n\n{contents}\n{AGENTS_MD_END_MARKER}", + directory = self.directory, + contents = self.text, + ) + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + _params: &TurnContextDiffParams<'_>, + ) -> Option { + let current = Self { + directory: turn_context.cwd.to_string_lossy().into_owned(), + text: turn_context.user_instructions.as_ref()?.clone(), + }; + if let Some(previous) = reference_context_item { + let previous_directory = previous.cwd.to_string_lossy().into_owned(); + if previous_directory == current.directory + && previous.user_instructions.as_deref() == Some(current.text.as_str()) + { + return None; + } + } + + Some(current) + } + + fn matches_contextual_user_text(text: &str) -> bool { + let trimmed = text.trim_start(); + // TODO(ccunningham): Switch detection to the XML-ish wrapper once we + // intentionally change the shipped AGENTS.md fragment format. + trimmed.starts_with(AGENTS_MD_START_MARKER) + && trimmed.trim_end().ends_with(AGENTS_MD_END_MARKER) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename = "environment_context", rename_all = "snake_case")] +pub(crate) struct EnvironmentContext { + pub cwd: Option, + pub shell: Shell, + pub current_date: Option, + pub timezone: Option, + pub network: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +pub(crate) struct NetworkContext { + allowed_domains: Vec, + denied_domains: Vec, +} + +impl EnvironmentContext { + const MARKERS: ContextualUserFragmentMarkers = ContextualUserFragmentMarkers::new( + ENVIRONMENT_CONTEXT_OPEN_TAG, + ENVIRONMENT_CONTEXT_CLOSE_TAG, + ); + + pub(crate) fn new( + cwd: Option, + shell: Shell, + current_date: Option, + timezone: Option, + network: Option, + ) -> Self { + Self { + cwd, + shell, + current_date, + timezone, + network, + } + } + + pub(crate) fn equals_except_shell(&self, other: &EnvironmentContext) -> bool { + let EnvironmentContext { + cwd, + current_date, + timezone, + network, + shell: _, + } = other; + self.cwd == *cwd + && self.current_date == *current_date + && self.timezone == *timezone + && self.network == *network + } + + fn network_from_turn_context(turn_context: &TurnContext) -> Option { + let network = turn_context + .config + .config_layer_stack + .requirements() + .network + .as_ref()?; + + Some(NetworkContext { + allowed_domains: network.allowed_domains.clone().unwrap_or_default(), + denied_domains: network.denied_domains.clone().unwrap_or_default(), + }) + } + + fn network_from_turn_context_item( + turn_context_item: &TurnContextItem, + ) -> Option { + let TurnContextNetworkItem { + allowed_domains, + denied_domains, + } = turn_context_item.network.as_ref()?; + Some(NetworkContext { + allowed_domains: allowed_domains.clone(), + denied_domains: denied_domains.clone(), + }) + } +} + +impl ModelVisibleContextFragment for EnvironmentContext { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + let mut lines = Vec::new(); + if let Some(cwd) = &self.cwd { + lines.push(format!(" {}", cwd.to_string_lossy())); + } + + let shell_name = self.shell.name(); + lines.push(format!(" {shell_name}")); + if let Some(current_date) = &self.current_date { + lines.push(format!(" {current_date}")); + } + if let Some(timezone) = &self.timezone { + lines.push(format!(" {timezone}")); + } + if let Some(network) = &self.network { + lines.push(" ".to_string()); + for allowed in &network.allowed_domains { + lines.push(format!(" {allowed}")); + } + for denied in &network.denied_domains { + lines.push(format!(" {denied}")); + } + lines.push(" ".to_string()); + } + Self::MARKERS.wrap_body(lines.join("\n")) + } + + fn build( + turn_context: &TurnContext, + reference_context_item: Option<&TurnContextItem>, + params: &TurnContextDiffParams<'_>, + ) -> Option { + let current_network = Self::network_from_turn_context(turn_context); + let current_context = Self::new( + Some(turn_context.cwd.clone()), + params.shell.clone(), + turn_context.current_date.clone(), + turn_context.timezone.clone(), + current_network.clone(), + ); + + let Some(previous) = reference_context_item else { + return Some(current_context); + }; + + let previous_network = Self::network_from_turn_context_item(previous); + let previous_context = Self::new( + Some(previous.cwd.clone()), + params.shell.clone(), + previous.current_date.clone(), + previous.timezone.clone(), + previous_network.clone(), + ); + + if previous_context.equals_except_shell(¤t_context) { + return None; + } + + let cwd = if previous.cwd != turn_context.cwd { + Some(turn_context.cwd.clone()) + } else { + None + }; + let network = if previous_network != current_network { + current_network + } else { + previous_network + }; + + Some(Self::new( + cwd, + params.shell.clone(), + turn_context.current_date.clone(), + turn_context.timezone.clone(), + network, + )) + } + + fn contextual_user_markers() -> Option { + Some(Self::MARKERS) + } +} + +// --------------------------------------------------------------------------- +// Contextual-user runtime fragments +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename = "skill_instructions", rename_all = "snake_case")] +pub(crate) struct SkillInstructions { + pub name: String, + pub path: String, + pub contents: String, +} + +impl ModelVisibleContextFragment for SkillInstructions { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + Self::wrap_contextual_user_body(format!( + "{}\n{}\n{}", + self.name, self.path, self.contents + )) + } + + fn contextual_user_markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + SKILL_OPEN_TAG, + SKILL_CLOSE_TAG, + )) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename = "plugin_instructions", rename_all = "snake_case")] +pub(crate) struct PluginInstructions { + pub text: String, +} + +impl ModelVisibleContextFragment for PluginInstructions { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + Self::wrap_contextual_user_body(self.text.clone()) + } + + fn contextual_user_markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + PLUGINS_OPEN_TAG, + PLUGINS_CLOSE_TAG, + )) + } +} + +pub(crate) struct UserShellCommandFragment { + text: String, +} + +impl UserShellCommandFragment { + pub(crate) fn from_exec_output( + command: &str, + exec_output: &ExecToolCallOutput, + turn_context: &TurnContext, + ) -> Self { + let mut sections = Vec::new(); + sections.push("".to_string()); + sections.push(command.to_string()); + sections.push("".to_string()); + sections.push("".to_string()); + sections.push(format!("Exit code: {}", exec_output.exit_code)); + sections.push(format_duration_line(exec_output.duration)); + sections.push("Output:".to_string()); + sections.push(format_exec_output_str( + exec_output, + turn_context.truncation_policy, + )); + sections.push("".to_string()); + + Self { + text: Self::wrap_contextual_user_body(sections.join("\n")), + } + } +} + +impl ModelVisibleContextFragment for UserShellCommandFragment { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + self.text.clone() + } + + fn contextual_user_markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + USER_SHELL_COMMAND_OPEN_TAG, + USER_SHELL_COMMAND_CLOSE_TAG, + )) + } +} + +pub(crate) struct TurnAbortedMarker { + guidance: &'static str, +} + +impl TurnAbortedMarker { + pub(crate) fn interrupted() -> Self { + Self { + guidance: "The user interrupted the previous turn on purpose. Any running unified exec processes were terminated. If any tools/commands were aborted, they may have partially executed; verify current state before retrying.", + } + } +} + +impl ModelVisibleContextFragment for TurnAbortedMarker { + type Role = ContextualUserContextRole; + + fn render_text(&self) -> String { + Self::wrap_contextual_user_body(self.guidance.to_string()) + } + + fn contextual_user_markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + TURN_ABORTED_OPEN_TAG, + TURN_ABORTED_CLOSE_TAG, + )) + } +} + +fn format_duration_line(duration: Duration) -> String { + let duration_seconds = duration.as_secs_f64(); + format!("Duration: {duration_seconds:.4} seconds") +} + +#[cfg(test)] +pub(crate) fn format_user_shell_command_record( + command: &str, + exec_output: &ExecToolCallOutput, + turn_context: &TurnContext, +) -> String { + UserShellCommandFragment::from_exec_output(command, exec_output, turn_context).render_text() +} + +pub(crate) fn user_shell_command_record_item( + command: &str, + exec_output: &ExecToolCallOutput, + turn_context: &TurnContext, +) -> ResponseItem { + UserShellCommandFragment::from_exec_output(command, exec_output, turn_context).into_message() +} + +// --------------------------------------------------------------------------- +// Shared fragment assembly and detection +// --------------------------------------------------------------------------- + +fn is_legacy_contextual_user_fragment(text: &str) -> bool { + // TODO(ccunningham): Drop this once old user-role subagent notification + // history no longer needs resume/compaction compatibility. + ContextualUserFragmentMarkers::new( + SUBAGENT_NOTIFICATION_OPEN_TAG, + SUBAGENT_NOTIFICATION_CLOSE_TAG, + ) + .matches_text(text) +} + +pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool { + let ContentItem::InputText { text } = content_item else { + return false; + }; + + REGISTERED_MODEL_VISIBLE_FRAGMENTS + .iter() + .any(|registration| (registration.detect_contextual_user)(text)) + || is_legacy_contextual_user_fragment(text) +} + +pub(crate) fn build_turn_state_fragments( + reference_context_item: Option<&TurnContextItem>, + turn_context: &TurnContext, + params: &TurnContextDiffParams<'_>, +) -> Vec { + REGISTERED_MODEL_VISIBLE_FRAGMENTS + .iter() + .filter_map(|registration| { + (registration.build_turn_state)(reference_context_item, turn_context, params) + }) + .collect() +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +#[path = "environment_context_tests.rs"] +mod environment_context_tests; + +#[cfg(test)] +#[path = "user_shell_command_tests.rs"] +mod user_shell_command_tests; + +#[cfg(test)] +mod tests { + use super::*; + use codex_protocol::models::ContentItem; + use pretty_assertions::assert_eq; + + #[test] + fn detects_environment_context_fragment() { + assert!(is_contextual_user_fragment(&ContentItem::InputText { + text: "\n/tmp\n".to_string(), + })); + } + + #[test] + fn detects_agents_instructions_fragment() { + assert!(is_contextual_user_fragment(&ContentItem::InputText { + text: "# AGENTS.md instructions for /tmp\n\n\nbody\n" + .to_string(), + })); + } + + #[test] + fn detects_legacy_subagent_notification_fragment() { + assert!(is_contextual_user_fragment(&ContentItem::InputText { + text: "\n{\"agent_id\":\"a\",\"status\":\"completed\"}\n" + .to_string(), + })); + } + + #[test] + fn ignores_regular_user_text() { + assert!(!is_contextual_user_fragment(&ContentItem::InputText { + text: "hello".to_string(), + })); + } + + #[test] + fn marker_matching_ignores_plain_text() { + assert!(!SkillInstructions::matches_contextual_user_text( + "plain text" + )); + } + + #[test] + fn serializes_subagent_roster_context() { + let context = + SubagentRosterContext::new("- agent-1: Atlas\n- agent-2: Juniper".to_string()) + .expect("context expected"); + + assert_eq!( + context.render_text(), + "\n - agent-1: Atlas\n - agent-2: Juniper\n" + ); + } + + #[test] + fn skips_empty_subagent_roster_context() { + assert!(SubagentRosterContext::new(String::new()).is_none()); + } +} diff --git a/codex-rs/core/src/plugins/render.rs b/codex-rs/core/src/plugins/render.rs index 076c4237f1..4ce5e53547 100644 --- a/codex-rs/core/src/plugins/render.rs +++ b/codex-rs/core/src/plugins/render.rs @@ -1,4 +1,4 @@ -use crate::instructions::PluginInstructions; +use crate::model_visible_fragments::PluginInstructions; use crate::plugins::PluginCapabilitySummary; pub(crate) fn render_plugin_instructions( diff --git a/codex-rs/core/src/session_prefix.rs b/codex-rs/core/src/session_prefix.rs deleted file mode 100644 index 5eb262098b..0000000000 --- a/codex-rs/core/src/session_prefix.rs +++ /dev/null @@ -1,94 +0,0 @@ -use codex_protocol::protocol::AgentStatus; - -/// Helpers for model-visible subagent session state rendered in the developer -/// envelope. -use crate::model_visible_context::DeveloperContextRole; -use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::SUBAGENT_NOTIFICATION_CLOSE_TAG; -use crate::model_visible_context::SUBAGENT_NOTIFICATION_OPEN_TAG; -use crate::model_visible_context::SUBAGENTS_CLOSE_TAG; -use crate::model_visible_context::SUBAGENTS_OPEN_TAG; - -pub(crate) struct SubagentRosterContext { - subagents: String, -} - -impl SubagentRosterContext { - pub(crate) fn new(subagents: String) -> Option { - if subagents.is_empty() { - None - } else { - Some(Self { subagents }) - } - } -} - -impl ModelVisibleContextFragment for SubagentRosterContext { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - let lines = self - .subagents - .lines() - .map(|line| format!(" {line}")) - .collect::>() - .join("\n"); - format!("{SUBAGENTS_OPEN_TAG}\n{lines}\n{SUBAGENTS_CLOSE_TAG}") - } -} - -pub(crate) struct SubagentNotification<'a> { - agent_id: &'a str, - status: &'a AgentStatus, -} - -impl<'a> SubagentNotification<'a> { - pub(crate) fn new(agent_id: &'a str, status: &'a AgentStatus) -> Self { - Self { agent_id, status } - } -} - -impl ModelVisibleContextFragment for SubagentNotification<'_> { - type Role = DeveloperContextRole; - - fn render_text(&self) -> String { - let payload_json = serde_json::json!({ - "agent_id": self.agent_id, - "status": self.status, - }) - .to_string(); - format!( - "{SUBAGENT_NOTIFICATION_OPEN_TAG}\n{payload_json}\n{SUBAGENT_NOTIFICATION_CLOSE_TAG}" - ) - } -} - -pub(crate) fn format_subagent_context_line(agent_id: &str, agent_nickname: Option<&str>) -> String { - match agent_nickname.filter(|nickname| !nickname.is_empty()) { - Some(agent_nickname) => format!("- {agent_id}: {agent_nickname}"), - None => format!("- {agent_id}"), - } -} - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - - #[test] - fn serializes_subagent_roster_context() { - let context = - SubagentRosterContext::new("- agent-1: Atlas\n- agent-2: Juniper".to_string()) - .expect("context expected"); - - assert_eq!( - context.render_text(), - "\n - agent-1: Atlas\n - agent-2: Juniper\n" - ); - } - - #[test] - fn skips_empty_subagent_roster_context() { - assert!(SubagentRosterContext::new(String::new()).is_none()); - } -} diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index e55745def4..7770290c6a 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -6,10 +6,10 @@ use crate::analytics_client::AnalyticsEventsClient; use crate::analytics_client::InvocationType; use crate::analytics_client::SkillInvocation; use crate::analytics_client::TrackEventsContext; -use crate::instructions::SkillInstructions; use crate::mention_syntax::TOOL_MENTION_SIGIL; use crate::mentions::build_skill_name_counts; use crate::model_visible_context::ModelVisibleContextFragment; +use crate::model_visible_fragments::SkillInstructions; use crate::skills::SkillMetadata; use codex_otel::SessionTelemetry; use codex_protocol::models::ResponseItem; diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index 4c844ddf1d..9be926f065 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -23,12 +23,8 @@ use crate::AuthManager; use crate::codex::Session; use crate::codex::TurnContext; use crate::event_mapping::parse_turn_item; -use crate::model_visible_context::ContextualUserContextRole; -use crate::model_visible_context::ContextualUserFragment; -use crate::model_visible_context::ContextualUserFragmentMarkers; use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::TURN_ABORTED_CLOSE_TAG; -use crate::model_visible_context::TURN_ABORTED_OPEN_TAG; +use crate::model_visible_fragments::TurnAbortedMarker; use crate::models_manager::manager::ModelsManager; use crate::protocol::EventMsg; use crate::protocol::TokenUsage; @@ -60,28 +56,6 @@ pub(crate) use user_shell::UserShellCommandTask; pub(crate) use user_shell::execute_user_shell_command; const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100; -const TURN_ABORTED_INTERRUPTED_GUIDANCE: &str = "The user interrupted the previous turn on purpose. Any running unified exec processes were terminated. If any tools/commands were aborted, they may have partially executed; verify current state before retrying."; - -pub(crate) struct TurnAbortedMarker { - guidance: &'static str, -} - -impl ModelVisibleContextFragment for TurnAbortedMarker { - type Role = ContextualUserContextRole; - - fn render_text(&self) -> String { - ::wrap_contextual_user_body(self.guidance.to_string()) - } -} - -impl ContextualUserFragment for TurnAbortedMarker { - fn markers() -> Option { - Some(ContextualUserFragmentMarkers::new( - TURN_ABORTED_OPEN_TAG, - TURN_ABORTED_CLOSE_TAG, - )) - } -} fn emit_turn_network_proxy_metric( session_telemetry: &SessionTelemetry, @@ -458,9 +432,7 @@ impl Session { if reason == TurnAbortReason::Interrupted { self.cleanup_after_interrupt(&task.turn_context).await; - let marker = TurnAbortedMarker { - guidance: TURN_ABORTED_INTERRUPTED_GUIDANCE, - }; + let marker = TurnAbortedMarker::interrupted(); let marker = marker.into_message(); self.record_into_history(std::slice::from_ref(&marker), task.turn_context.as_ref()) .await; diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 64fe70491d..f2333d4d7e 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -16,6 +16,7 @@ use crate::exec::StdoutStream; use crate::exec::StreamOutput; use crate::exec::execute_exec_request; use crate::exec_env::create_env; +use crate::model_visible_fragments::user_shell_command_record_item; use crate::parse_command::parse_command; use crate::protocol::EventMsg; use crate::protocol::ExecCommandBeginEvent; @@ -29,7 +30,6 @@ use crate::sandboxing::SandboxPermissions; use crate::state::TaskKind; use crate::tools::format_exec_output_str; use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot; -use crate::user_shell_command::user_shell_command_record_item; use super::SessionTask; use super::SessionTaskContext; diff --git a/codex-rs/core/src/user_shell_command.rs b/codex-rs/core/src/user_shell_command.rs deleted file mode 100644 index 360e0709da..0000000000 --- a/codex-rs/core/src/user_shell_command.rs +++ /dev/null @@ -1,89 +0,0 @@ -use std::time::Duration; - -use codex_protocol::models::ResponseItem; - -use crate::codex::TurnContext; -use crate::exec::ExecToolCallOutput; -use crate::model_visible_context::ContextualUserContextRole; -use crate::model_visible_context::ContextualUserFragment; -use crate::model_visible_context::ContextualUserFragmentMarkers; -use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::USER_SHELL_COMMAND_CLOSE_TAG; -use crate::model_visible_context::USER_SHELL_COMMAND_OPEN_TAG; -use crate::tools::format_exec_output_str; - -fn format_duration_line(duration: Duration) -> String { - let duration_seconds = duration.as_secs_f64(); - format!("Duration: {duration_seconds:.4} seconds") -} - -pub(crate) struct UserShellCommandFragment; - -impl ContextualUserFragment for UserShellCommandFragment { - fn markers() -> Option { - Some(ContextualUserFragmentMarkers::new( - USER_SHELL_COMMAND_OPEN_TAG, - USER_SHELL_COMMAND_CLOSE_TAG, - )) - } -} - -struct UserShellCommandRecord<'a> { - command: &'a str, - exec_output: &'a ExecToolCallOutput, - turn_context: &'a TurnContext, -} - -impl ModelVisibleContextFragment for UserShellCommandRecord<'_> { - type Role = ContextualUserContextRole; - - fn render_text(&self) -> String { - let mut sections = Vec::new(); - sections.push("".to_string()); - sections.push(self.command.to_string()); - sections.push("".to_string()); - sections.push("".to_string()); - sections.push(format!("Exit code: {}", self.exec_output.exit_code)); - sections.push(format_duration_line(self.exec_output.duration)); - sections.push("Output:".to_string()); - sections.push(format_exec_output_str( - self.exec_output, - self.turn_context.truncation_policy, - )); - sections.push("".to_string()); - ::wrap_contextual_user_body( - sections.join("\n"), - ) - } -} - -#[cfg(test)] -pub fn format_user_shell_command_record( - command: &str, - exec_output: &ExecToolCallOutput, - turn_context: &TurnContext, -) -> String { - UserShellCommandRecord { - command, - exec_output, - turn_context, - } - .render_text() -} - -pub fn user_shell_command_record_item( - command: &str, - exec_output: &ExecToolCallOutput, - turn_context: &TurnContext, -) -> ResponseItem { - UserShellCommandRecord { - command, - exec_output, - turn_context, - } - .into_message() -} - -#[cfg(test)] -#[path = "user_shell_command_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/user_shell_command_tests.rs b/codex-rs/core/src/user_shell_command_tests.rs index 4b30865f04..d33079167d 100644 --- a/codex-rs/core/src/user_shell_command_tests.rs +++ b/codex-rs/core/src/user_shell_command_tests.rs @@ -7,10 +7,10 @@ use pretty_assertions::assert_eq; #[test] fn detects_user_shell_command_text_variants() { assert!( - ::matches_contextual_user_text("\necho hi\n") + ::matches_contextual_user_text("\necho hi\n") ); assert!( - !::matches_contextual_user_text("echo hi") + !::matches_contextual_user_text("echo hi") ); } diff --git a/docs/model-visible-context.md b/docs/model-visible-context.md index 6c91f32d0c..c79064762b 100644 --- a/docs/model-visible-context.md +++ b/docs/model-visible-context.md @@ -1,43 +1,52 @@ # Model-visible context fragments -Codex injects model-visible context through two envelopes: +Codex injects model-visible context through two turn-state envelopes: - the developer envelope, rendered as a single `developer` message - the contextual-user envelope, rendered as a single `user` message whose contents are contextual state rather than real user intent -Both envelopes use the same internal fragment contract in `codex-rs/core`. -Envelope builders normalize text fragment boundaries by inserting `\n\n` between adjacent text content items, so fragments do not run together in the model-visible token stream. +Both envelopes are assembled from the single ordered fragment registry in +[`codex-rs/core/src/model_visible_fragments.rs`](/Users/ccunningham/code/codex-worktree-tria/codex-rs/core/src/model_visible_fragments.rs). +Envelope builders normalize text fragment boundaries by inserting `\n\n` +between adjacent text content items, so fragments do not run together in the +model-visible token stream. ## Canonical rules 1. All model-visible injected context must be represented as a typed `ModelVisibleContextFragment`. 2. Turn-state context assembly always produces exactly two envelopes: one developer message and one contextual-user message. -3. Contextual-user fragments must have stable detection so history parsing can distinguish contextual state from true user intent. -4. If a fragment is derived from durable/current turn state and should survive history-mutating flows (resume/fork/compaction/backtracking) via re-diffing, it must implement `TurnContextDiffFragment`. -5. Do not hand-construct model-visible `ResponseItem::Message` payloads in new code. Use fragment conversion (`into_message` / `into_response_input_item`) and envelope builders. +3. There is one blessed place to define and register current fragment types: [`model_visible_fragments.rs`](/Users/ccunningham/code/codex-worktree-tria/codex-rs/core/src/model_visible_fragments.rs). +4. Contextual-user fragments must provide stable detection so history parsing can distinguish contextual state from true user intent. +5. Fragments derived from durable/current turn state that should survive history-mutating flows (resume/fork/compaction/backtracking) via re-diffing should implement `ModelVisibleContextFragment::build(...)`. +6. Do not hand-construct model-visible `ResponseItem::Message` payloads in new code. Use fragment conversion (`into_message` / `into_response_input_item`) and the shared envelope builders. -Rule 2 applies to turn-state context assembly (`build_initial_context` / `build_settings_update_items`). Runtime or session-prefix events may inject standalone fragment messages, but those still must be typed `ModelVisibleContextFragment`s (rules 1 and 5). +Rule 2 applies to turn-state context assembly (`build_initial_context` / +`build_settings_update_items`). Runtime or session-prefix events may still +inject standalone fragment messages, but those fragment types still belong in +the central registry so rendering and contextual-user detection remain +standardized. ## Blessed path When adding new model-visible context: -1. Define a typed fragment type. +1. Define a typed fragment type in [`model_visible_fragments.rs`](/Users/ccunningham/code/codex-worktree-tria/codex-rs/core/src/model_visible_fragments.rs). 2. Implement `ModelVisibleContextFragment` for it. 3. Set the fragment `type Role` to the correct developer or contextual-user role. -4. If it is a contextual-user fragment, implement contextual-user detection: - - implement `ContextualUserFragment` - - prefer defining `markers()` so the default marker-based detection/wrapping applies +4. If it is a contextual-user fragment: + - define `contextual_user_markers()` when marker-based detection/wrapping is sufficient - override `matches_contextual_user_text()` only when matching is genuinely custom (for example AGENTS.md) -5. If the fragment is derived from durable/current turn state and should be diffed/reinjected after history mutations, also implement `TurnContextDiffFragment`. -6. Register the fragment type: - - developer turn-state fragments: add to `REGISTERED_TURN_STATE_FRAGMENT_BUILDERS` in `context_manager/updates.rs`. - - contextual-user fragments: add once to `REGISTERED_CONTEXTUAL_USER_FRAGMENTS` in `model_visible_context.rs` (this powers both history detection and optional turn-state diff assembly). -7. Push the resulting fragments through the shared envelope builders. +5. If it is derived from current/persisted turn state and should participate in initial-context assembly and turn-to-turn diffing, implement `build(...)`. +6. Register the fragment exactly once in `REGISTERED_MODEL_VISIBLE_FRAGMENTS`, in the rough order it should appear in model-visible context. -Do not hand-build developer or contextual-user model-visible `ResponseItem`s in new code. +That single registration powers: -The role lives in the fragment's associated `type Role`. +- contextual-user history detection +- developer-envelope turn-state assembly +- contextual-user-envelope turn-state assembly + +This is intentionally stricter than “implement the trait somewhere.” A fragment +definition is not integrated until it is registered. ## Choosing an envelope @@ -50,46 +59,66 @@ Use the developer envelope for developer-role guidance: - subagent roster and subagent notifications - other developer-only instructions -Use `CustomDeveloperInstructions` only for custom developer override text (for example config/app-server `developer_instructions` values). +Use `CustomDeveloperInstructions` only for custom developer override text (for +example config/app-server `developer_instructions` values). -For system-generated developer guidance (permissions, collaboration-mode wrappers, realtime notices, personality wrappers, model-switch notices), use typed developer fragments whose text comes from the neutral `developer_*_text` helpers in `codex_protocol::models`. +For system-generated developer guidance (permissions, collaboration-mode +wrappers, realtime notices, personality wrappers, model-switch notices), use +typed developer fragments whose text comes from the neutral +`developer_*_text` helpers in `codex_protocol::models`. -Use the contextual-user envelope for contextual state or runtime markers that should not count as real user turns: +Use the contextual-user envelope for contextual state that should not count as +real user turns: - AGENTS / user instructions -- plugin instructions - environment context + +Use standalone contextual-user fragment messages for runtime contextual state or +markers that also should not count as real user turns: + +- plugin instructions - skill instructions - user shell command records - turn-aborted markers -Contextual-user fragments must have stable detection because history parsing uses it to distinguish contextual state from real user intent. +Contextual-user fragments must have stable detection because history parsing +uses it to distinguish contextual state from real user intent. -Use `` only for environment facts derived from turn/session state (`TurnContext`) that may need turn-to-turn diffing. Today that includes `cwd`, `shell`, optional `current_date`, optional `timezone`, and optional network allow/deny domain summaries. Do not put developer policy/instructions or plugin/skill metadata into ``; those belong in their own typed fragments. +Use `` only for environment facts derived from turn/session +state (`TurnContext`) that may need turn-to-turn diffing. Today that includes +`cwd`, `shell`, optional `current_date`, optional `timezone`, and optional +network allow/deny domain summaries. Do not put developer policy/instructions +or plugin/skill metadata into ``; those belong in their +own typed fragments. -## Turn-backed fragments +## Build semantics -If a fragment is derived from durable turn/session state and should be updated/reinjected by diff after history mutation, keep its extraction, diffing, and rendering logic together by implementing `TurnContextDiffFragment`. +`ModelVisibleContextFragment::build(...)` is the canonical hook for turn-state +fragments. -`TurnContextDiffFragment` exposes one `build(...)` method that receives: +It receives: -- current `TurnContext` -- optional `reference_context_item` (the turn context state already represented in model-visible history, if available) -- `TurnContextDiffParams` for shared runtime inputs (for example shell, previous-turn bridge state, exec-policy rendering context, and feature gating flags) +- the current `TurnContext` +- an optional `reference_context_item`, which is the last persisted turn-state snapshot already represented in model-visible history +- `TurnContextDiffParams` for shared runtime inputs such as shell rendering, previous-turn bridge state, exec-policy rendering context, and feature gating flags -This is envelope-agnostic: both contextual-user state fragments and developer state-diff fragments use the same trait. +Turn-state fragments should return: -If a fragment is runtime-event/session-prefix only (for example subagent completion notification, turn-aborted marker, or user-shell-command marker), `ModelVisibleContextFragment` alone is enough. +- `Some(fragment)` when current model-visible state should be injected +- `None` when no model-visible update is needed -That trait is the blessed path for fragments that need to: - -- build full initial context when no reference context item is available -- compute turn diffs when a reference context item is available - -`EnvironmentContext` is the canonical example. Future turn-backed contextual fragments should follow the same pattern instead of introducing one-off extraction or diff helpers. +Runtime/session-prefix fragments that are not built from turn state should leave +the default `build(...) -> None`. ## History behavior -Developer fragments do not need contextual-user marker matching because they are already separable by message role. +Developer fragments do not need contextual-user detection because they are +already separable by message role. -Contextual-user fragments do need marker matching because they share the `user` role with real user turns, and history parsing / truncation must avoid treating injected context as actual user input. +Contextual-user fragments do need contextual-user detection because they share +the `user` role with real user turns, and history parsing / truncation must +avoid treating injected context as actual user input. + +Current fragment types live in the registry. Historical wrappers that are no +longer current fragments should stay in a tiny separate compatibility shim near +the detection path rather than being added as fake current fragments.