From 4c2e730488997145ad9bfa07f193293aeb30c083 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 20 Apr 2026 22:39:17 -0700 Subject: [PATCH] Organize context fragments (#18794) Organize context fragments under `core/context`. Implement same trait on all of them. --- codex-rs/Cargo.lock | 11 -- codex-rs/Cargo.toml | 2 - codex-rs/core-skills/Cargo.toml | 1 - codex-rs/core-skills/src/injection.rs | 15 +- codex-rs/core/Cargo.toml | 1 - codex-rs/core/src/agent/control_tests.rs | 5 +- codex-rs/core/src/arc_monitor_tests.rs | 15 +- .../{ => context}/contextual_user_message.rs | 73 +++++----- .../contextual_user_message_tests.rs | 8 +- .../src/{ => context}/environment_context.rs | 133 ++++++++--------- .../environment_context_tests.rs | 134 +++--------------- codex-rs/core/src/context/fragment.rs | 82 +++++++++++ codex-rs/core/src/context/mod.rs | 21 +++ .../core/src/context/skill_instructions.rs | 33 +++++ .../core/src/context/subagent_notification.rs | 34 +++++ codex-rs/core/src/context/turn_aborted.rs | 26 ++++ .../core/src/context/user_instructions.rs | 17 +++ .../core/src/context/user_shell_command.rs | 40 ++++++ codex-rs/core/src/context_manager/updates.rs | 9 +- codex-rs/core/src/event_mapping.rs | 4 +- codex-rs/core/src/instructions/mod.rs | 1 - codex-rs/core/src/lib.rs | 4 +- codex-rs/core/src/memories/phase1.rs | 2 +- codex-rs/core/src/session/mod.rs | 10 +- codex-rs/core/src/session/tests.rs | 4 +- codex-rs/core/src/session/turn.rs | 8 +- codex-rs/core/src/session_prefix.rs | 15 +- codex-rs/core/src/tasks/mod.rs | 19 +-- codex-rs/core/src/user_shell_command.rs | 36 ++--- codex-rs/core/src/user_shell_command_tests.rs | 12 +- codex-rs/instructions/BUILD.bazel | 16 --- codex-rs/instructions/Cargo.toml | 20 --- codex-rs/instructions/src/fragment.rs | 61 -------- codex-rs/instructions/src/lib.rs | 11 -- .../instructions/src/user_instructions.rs | 56 -------- .../src/user_instructions_tests.rs | 72 ---------- 36 files changed, 452 insertions(+), 559 deletions(-) rename codex-rs/core/src/{ => context}/contextual_user_message.rs (50%) rename codex-rs/core/src/{ => context}/contextual_user_message_tests.rs (94%) rename codex-rs/core/src/{ => context}/environment_context.rs (61%) rename codex-rs/core/src/{ => context}/environment_context_tests.rs (56%) create mode 100644 codex-rs/core/src/context/fragment.rs create mode 100644 codex-rs/core/src/context/mod.rs create mode 100644 codex-rs/core/src/context/skill_instructions.rs create mode 100644 codex-rs/core/src/context/subagent_notification.rs create mode 100644 codex-rs/core/src/context/turn_aborted.rs create mode 100644 codex-rs/core/src/context/user_instructions.rs create mode 100644 codex-rs/core/src/context/user_shell_command.rs delete mode 100644 codex-rs/core/src/instructions/mod.rs delete mode 100644 codex-rs/instructions/BUILD.bazel delete mode 100644 codex-rs/instructions/Cargo.toml delete mode 100644 codex-rs/instructions/src/fragment.rs delete mode 100644 codex-rs/instructions/src/lib.rs delete mode 100644 codex-rs/instructions/src/user_instructions.rs delete mode 100644 codex-rs/instructions/src/user_instructions_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 72d23456b2..15ca924a8b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1941,7 +1941,6 @@ dependencies = [ "codex-feedback", "codex-git-utils", "codex-hooks", - "codex-instructions", "codex-login", "codex-mcp", "codex-model-provider", @@ -2074,7 +2073,6 @@ dependencies = [ "codex-app-server-protocol", "codex-config", "codex-exec-server", - "codex-instructions", "codex-login", "codex-otel", "codex-protocol", @@ -2319,15 +2317,6 @@ dependencies = [ "tempfile", ] -[[package]] -name = "codex-instructions" -version = "0.0.0" -dependencies = [ - "codex-protocol", - "pretty_assertions", - "serde", -] - [[package]] name = "codex-keyring-store" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 4c15e47074..19987fb8f0 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -31,7 +31,6 @@ members = [ "core-plugins", "core-skills", "hooks", - "instructions", "secrets", "exec", "exec-server", @@ -143,7 +142,6 @@ codex-install-context = { path = "install-context" } codex-file-search = { path = "file-search" } codex-git-utils = { path = "git-utils" } codex-hooks = { path = "hooks" } -codex-instructions = { path = "instructions" } codex-keyring-store = { path = "keyring-store" } codex-linux-sandbox = { path = "linux-sandbox" } codex-lmstudio = { path = "lmstudio" } diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index 09278c8d0e..355374114a 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -18,7 +18,6 @@ codex-analytics = { workspace = true } codex-app-server-protocol = { workspace = true } codex-config = { workspace = true } codex-exec-server = { workspace = true } -codex-instructions = { workspace = true } codex-login = { workspace = true } codex-otel = { workspace = true } codex-protocol = { workspace = true } diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index 1ccd447cf0..ed06cc578e 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -10,19 +10,24 @@ use codex_analytics::InvocationType; use codex_analytics::SkillInvocation; use codex_analytics::TrackEventsContext; use codex_exec_server::LOCAL_FS; -use codex_instructions::SkillInstructions; use codex_otel::SessionTelemetry; -use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; #[derive(Debug, Default)] pub struct SkillInjections { - pub items: Vec, + pub items: Vec, pub warnings: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillInjection { + pub name: String, + pub path: String, + pub contents: String, +} + pub async fn build_skill_injections( mentioned_skills: &[SkillMetadata], loaded_skills: Option<&SkillLoadOutcome>, @@ -56,11 +61,11 @@ pub async fn build_skill_injections( skill_path: skill.path_to_skills_md.to_path_buf(), invocation_type: InvocationType::Explicit, }); - result.items.push(ResponseItem::from(SkillInstructions { + result.items.push(SkillInjection { name: skill.name.clone(), path: skill.path_to_skills_md.to_string_lossy().into_owned(), contents, - })); + }); } Err(err) => { emit_skill_injected_metric(otel, skill, "error"); diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 8770aaf487..6ef6741fd9 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -48,7 +48,6 @@ codex-shell-command = { workspace = true } codex-execpolicy = { workspace = true } codex-git-utils = { workspace = true } codex-hooks = { workspace = true } -codex-instructions = { workspace = true } codex-network-proxy = { workspace = true } codex-otel = { workspace = true } codex-plugin = { workspace = true } diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index b444d8c67c..10850ef8c7 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -5,7 +5,8 @@ use crate::agent::agent_status_from_event; use crate::config::AgentRoleConfig; use crate::config::Config; use crate::config::ConfigBuilder; -use crate::contextual_user_message::SUBAGENT_NOTIFICATION_OPEN_TAG; +use crate::context::ContextualUserFragment; +use crate::context::SubagentNotification; use assert_matches::assert_matches; use codex_features::Feature; use codex_login::CodexAuth; @@ -127,7 +128,7 @@ fn has_subagent_notification(history_items: &[ResponseItem]) -> bool { } content.iter().any(|content_item| match content_item { ContentItem::InputText { text } | ContentItem::OutputText { text } => { - text.contains(SUBAGENT_NOTIFICATION_OPEN_TAG) + SubagentNotification::matches_text(text) } ContentItem::InputImage { .. } => false, }) diff --git a/codex-rs/core/src/arc_monitor_tests.rs b/codex-rs/core/src/arc_monitor_tests.rs index 6bf1aa98f0..5fd691b930 100644 --- a/codex-rs/core/src/arc_monitor_tests.rs +++ b/codex-rs/core/src/arc_monitor_tests.rs @@ -1,5 +1,6 @@ use std::env; use std::ffi::OsStr; +use std::path::PathBuf; use std::sync::Arc; use pretty_assertions::assert_eq; @@ -16,6 +17,7 @@ use wiremock::matchers::path; use super::*; use crate::agent_identity::AgentIdentityManager; use crate::agent_identity::RegisteredAgentTask; +use crate::context::ContextualUserFragment; use crate::session::tests::make_session_and_context; use chrono::Utc; use codex_login::AuthCredentialsStoreMode; @@ -143,11 +145,16 @@ async fn build_arc_monitor_request_includes_relevant_history_and_null_policies() .await; session .record_into_history( - &[ - crate::contextual_user_message::ENVIRONMENT_CONTEXT_FRAGMENT.into_message( - "\n/tmp\n".to_string(), + &[ContextualUserFragment::into( + crate::context::EnvironmentContext::new( + Some(PathBuf::from("/tmp")), + "zsh".to_string(), + /*current_date*/ None, + /*timezone*/ None, + /*network*/ None, + /*subagents*/ None, ), - ], + )], &turn_context, ) .await; diff --git a/codex-rs/core/src/contextual_user_message.rs b/codex-rs/core/src/context/contextual_user_message.rs similarity index 50% rename from codex-rs/core/src/contextual_user_message.rs rename to codex-rs/core/src/context/contextual_user_message.rs index 68a86cc885..aeb54d61d7 100644 --- a/codex-rs/core/src/contextual_user_message.rs +++ b/codex-rs/core/src/context/contextual_user_message.rs @@ -1,50 +1,47 @@ -use codex_instructions::AGENTS_MD_FRAGMENT; -use codex_instructions::ContextualUserFragmentDefinition; -use codex_instructions::SKILL_FRAGMENT; use codex_protocol::items::HookPromptItem; use codex_protocol::items::parse_hook_prompt_fragment; use codex_protocol::models::ContentItem; -use codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG; -use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; -pub(crate) const USER_SHELL_COMMAND_OPEN_TAG: &str = ""; -pub(crate) const USER_SHELL_COMMAND_CLOSE_TAG: &str = ""; -pub(crate) const TURN_ABORTED_OPEN_TAG: &str = ""; -pub(crate) const TURN_ABORTED_CLOSE_TAG: &str = ""; -pub(crate) const SUBAGENT_NOTIFICATION_OPEN_TAG: &str = ""; -pub(crate) const SUBAGENT_NOTIFICATION_CLOSE_TAG: &str = ""; +use super::EnvironmentContext; +use super::FragmentRegistration; +use super::FragmentRegistrationProxy; +use super::SkillInstructions; +use super::SubagentNotification; +use super::TurnAborted; +use super::UserInstructions; +use super::UserShellCommand; -pub(crate) const ENVIRONMENT_CONTEXT_FRAGMENT: ContextualUserFragmentDefinition = - ContextualUserFragmentDefinition::new( - ENVIRONMENT_CONTEXT_OPEN_TAG, - ENVIRONMENT_CONTEXT_CLOSE_TAG, - ); -pub(crate) const USER_SHELL_COMMAND_FRAGMENT: ContextualUserFragmentDefinition = - ContextualUserFragmentDefinition::new( - USER_SHELL_COMMAND_OPEN_TAG, - USER_SHELL_COMMAND_CLOSE_TAG, - ); -pub(crate) const TURN_ABORTED_FRAGMENT: ContextualUserFragmentDefinition = - ContextualUserFragmentDefinition::new(TURN_ABORTED_OPEN_TAG, TURN_ABORTED_CLOSE_TAG); -pub(crate) const SUBAGENT_NOTIFICATION_FRAGMENT: ContextualUserFragmentDefinition = - ContextualUserFragmentDefinition::new( - SUBAGENT_NOTIFICATION_OPEN_TAG, - SUBAGENT_NOTIFICATION_CLOSE_TAG, - ); +static USER_INSTRUCTIONS_REGISTRATION: FragmentRegistrationProxy = + FragmentRegistrationProxy::new(); +static ENVIRONMENT_CONTEXT_REGISTRATION: FragmentRegistrationProxy = + FragmentRegistrationProxy::new(); +static SKILL_INSTRUCTIONS_REGISTRATION: FragmentRegistrationProxy = + FragmentRegistrationProxy::new(); +static USER_SHELL_COMMAND_REGISTRATION: FragmentRegistrationProxy = + FragmentRegistrationProxy::new(); +static TURN_ABORTED_REGISTRATION: FragmentRegistrationProxy = + FragmentRegistrationProxy::new(); +static SUBAGENT_NOTIFICATION_REGISTRATION: FragmentRegistrationProxy = + FragmentRegistrationProxy::new(); -const CONTEXTUAL_USER_FRAGMENTS: &[ContextualUserFragmentDefinition] = &[ - AGENTS_MD_FRAGMENT, - ENVIRONMENT_CONTEXT_FRAGMENT, - SKILL_FRAGMENT, - USER_SHELL_COMMAND_FRAGMENT, - TURN_ABORTED_FRAGMENT, - SUBAGENT_NOTIFICATION_FRAGMENT, +static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[ + &USER_INSTRUCTIONS_REGISTRATION, + &ENVIRONMENT_CONTEXT_REGISTRATION, + &SKILL_INSTRUCTIONS_REGISTRATION, + &USER_SHELL_COMMAND_REGISTRATION, + &TURN_ABORTED_REGISTRATION, + &SUBAGENT_NOTIFICATION_REGISTRATION, +]; + +static MEMORY_EXCLUDED_CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[ + &USER_INSTRUCTIONS_REGISTRATION, + &SKILL_INSTRUCTIONS_REGISTRATION, ]; fn is_standard_contextual_user_text(text: &str) -> bool { CONTEXTUAL_USER_FRAGMENTS .iter() - .any(|definition| definition.matches_text(text)) + .any(|fragment| fragment.matches_text(text)) } /// Returns whether a contextual user fragment should be omitted from memory @@ -59,7 +56,9 @@ pub(crate) fn is_memory_excluded_contextual_user_fragment(content_item: &Content let ContentItem::InputText { text } = content_item else { return false; }; - AGENTS_MD_FRAGMENT.matches_text(text) || SKILL_FRAGMENT.matches_text(text) + MEMORY_EXCLUDED_CONTEXTUAL_USER_FRAGMENTS + .iter() + .any(|fragment| fragment.matches_text(text)) } pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool { diff --git a/codex-rs/core/src/contextual_user_message_tests.rs b/codex-rs/core/src/context/contextual_user_message_tests.rs similarity index 94% rename from codex-rs/core/src/contextual_user_message_tests.rs rename to codex-rs/core/src/context/contextual_user_message_tests.rs index 406f03c27f..d52cf27adb 100644 --- a/codex-rs/core/src/contextual_user_message_tests.rs +++ b/codex-rs/core/src/context/contextual_user_message_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::context::ContextualUserFragment; use codex_protocol::items::HookPromptFragment; use codex_protocol::items::build_hook_prompt_message; use codex_protocol::models::ResponseItem; @@ -20,10 +21,9 @@ fn detects_agents_instructions_fragment() { #[test] fn detects_subagent_notification_fragment_case_insensitively() { - assert!( - SUBAGENT_NOTIFICATION_FRAGMENT - .matches_text("{}") - ); + assert!(SubagentNotification::matches_text( + "{}" + )); } #[test] diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/context/environment_context.rs similarity index 61% rename from codex-rs/core/src/environment_context.rs rename to codex-rs/core/src/context/environment_context.rs index 44929b8947..c676bfb9d9 100644 --- a/codex-rs/core/src/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -1,34 +1,40 @@ -use crate::contextual_user_message::ENVIRONMENT_CONTEXT_FRAGMENT; use crate::session::turn_context::TurnContext; use crate::shell::Shell; -use codex_protocol::models::ResponseItem; 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")] +use super::ContextualUserFragment; + +#[derive(Debug, Clone, PartialEq)] pub(crate) struct EnvironmentContext { - pub cwd: Option, - pub shell: Shell, - pub current_date: Option, - pub timezone: Option, - pub network: Option, - pub subagents: Option, + pub(crate) cwd: Option, + pub(crate) shell: String, + pub(crate) current_date: Option, + pub(crate) timezone: Option, + pub(crate) network: Option, + pub(crate) subagents: Option, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +#[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct NetworkContext { allowed_domains: Vec, denied_domains: Vec, } +impl NetworkContext { + pub(crate) fn new(allowed_domains: Vec, denied_domains: Vec) -> Self { + Self { + allowed_domains, + denied_domains, + } + } +} + impl EnvironmentContext { - pub fn new( + pub(crate) fn new( cwd: Option, - shell: Shell, + shell: String, current_date: Option, timezone: Option, network: Option, @@ -47,7 +53,7 @@ impl EnvironmentContext { /// 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 { + pub(crate) fn equals_except_shell(&self, other: &EnvironmentContext) -> bool { let EnvironmentContext { cwd, current_date, @@ -63,39 +69,34 @@ impl EnvironmentContext { && self.subagents == *subagents } - pub fn diff_from_turn_context_item( + pub(crate) fn diff_from_turn_context_item( before: &TurnContextItem, - after: &TurnContext, - shell: &Shell, + after: &EnvironmentContext, ) -> Self { let before_network = Self::network_from_turn_context_item(before); - let after_network = Self::network_from_turn_context(after); - let cwd = if before.cwd.as_path() != after.cwd.as_path() { - Some(after.cwd.to_path_buf()) - } else { - None + let cwd = match &after.cwd { + Some(cwd) if before.cwd.as_path() != cwd.as_path() => Some(cwd.clone()), + _ => None, }; - let current_date = after.current_date.clone(); - let timezone = after.timezone.clone(); - let network = if before_network != after_network { - after_network + let network = if before_network != after.network { + after.network.clone() } else { before_network }; EnvironmentContext::new( cwd, - shell.clone(), - current_date, - timezone, + after.shell.clone(), + after.current_date.clone(), + after.timezone.clone(), network, /*subagents*/ None, ) } - pub fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self { + pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self { Self::new( Some(turn_context.cwd.to_path_buf()), - shell.clone(), + shell.name().to_string(), turn_context.current_date.clone(), turn_context.timezone.clone(), Self::network_from_turn_context(turn_context), @@ -103,10 +104,13 @@ impl EnvironmentContext { ) } - pub fn from_turn_context_item(turn_context_item: &TurnContextItem, shell: &Shell) -> Self { + pub(crate) fn from_turn_context_item( + turn_context_item: &TurnContextItem, + shell: String, + ) -> Self { Self::new( Some(turn_context_item.cwd.clone()), - shell.clone(), + shell, turn_context_item.current_date.clone(), turn_context_item.timezone.clone(), Self::network_from_turn_context_item(turn_context_item), @@ -114,7 +118,7 @@ impl EnvironmentContext { ) } - pub fn with_subagents(mut self, subagents: String) -> Self { + pub(crate) fn with_subagents(mut self, subagents: String) -> Self { if !subagents.is_empty() { self.subagents = Some(subagents); } @@ -129,18 +133,18 @@ impl EnvironmentContext { .network .as_ref()?; - Some(NetworkContext { - allowed_domains: network + Some(NetworkContext::new( + network .domains .as_ref() .and_then(codex_config::NetworkDomainPermissionsToml::allowed_domains) .unwrap_or_default(), - denied_domains: network + network .domains .as_ref() .and_then(codex_config::NetworkDomainPermissionsToml::denied_domains) .unwrap_or_default(), - }) + )) } fn network_from_turn_context_item( @@ -150,40 +154,33 @@ impl EnvironmentContext { allowed_domains, denied_domains, } = turn_context_item.network.as_ref()?; - Some(NetworkContext { - allowed_domains: allowed_domains.clone(), - denied_domains: denied_domains.clone(), - }) + Some(NetworkContext::new( + allowed_domains.clone(), + denied_domains.clone(), + )) } } -impl EnvironmentContext { - /// Serializes the environment context to XML. Libraries like `quick-xml` - /// require custom macros to handle Enums with newtypes, so we just do it - /// manually, to keep things simple. Output looks like: - /// - /// ```xml - /// - /// ... - /// ... - /// - /// ``` - pub fn serialize_to_xml(self) -> String { +impl ContextualUserFragment for EnvironmentContext { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; + const END_MARKER: &'static str = codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG; + + fn body(&self) -> String { let mut lines = Vec::new(); - if let Some(cwd) = self.cwd { + 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!(" {}", self.shell)); + if let Some(current_date) = &self.current_date { lines.push(format!(" {current_date}")); } - if let Some(timezone) = self.timezone { + if let Some(timezone) = &self.timezone { lines.push(format!(" {timezone}")); } - match self.network { - Some(ref network) => { + match &self.network { + Some(network) => { lines.push(" ".to_string()); for allowed in &network.allowed_domains { lines.push(format!(" {allowed}")); @@ -198,18 +195,12 @@ impl EnvironmentContext { // lines.push(" ".to_string()); } } - if let Some(subagents) = self.subagents { + if let Some(subagents) = &self.subagents { lines.push(" ".to_string()); lines.extend(subagents.lines().map(|line| format!(" {line}"))); lines.push(" ".to_string()); } - ENVIRONMENT_CONTEXT_FRAGMENT.wrap(lines.join("\n")) - } -} - -impl From for ResponseItem { - fn from(ec: EnvironmentContext) -> Self { - ENVIRONMENT_CONTEXT_FRAGMENT.into_message(ec.serialize_to_xml()) + format!("\n{}", lines.join("\n")) } } diff --git a/codex-rs/core/src/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs similarity index 56% rename from codex-rs/core/src/environment_context_tests.rs rename to codex-rs/core/src/context/environment_context_tests.rs index 073f1fe169..84f8c0d99f 100644 --- a/codex-rs/core/src/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -3,13 +3,15 @@ use crate::shell::ShellType; use super::*; use core_test_support::test_path_buf; use pretty_assertions::assert_eq; +use std::path::PathBuf; -fn fake_shell() -> Shell { - Shell { +fn fake_shell_name() -> String { + let shell = crate::shell::Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), - } + }; + shell.name().to_string() } #[test] @@ -17,7 +19,7 @@ fn serialize_workspace_write_environment_context() { let cwd = test_path_buf("/repo"); let context = EnvironmentContext::new( Some(cwd.clone()), - fake_shell(), + fake_shell_name(), Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), /*network*/ None, @@ -34,18 +36,18 @@ fn serialize_workspace_write_environment_context() { cwd = cwd.display(), ); - assert_eq!(context.serialize_to_xml(), expected); + assert_eq!(context.render(), expected); } #[test] fn serialize_environment_context_with_network() { - let network = NetworkContext { - allowed_domains: vec!["api.example.com".to_string(), "*.openai.com".to_string()], - denied_domains: vec!["blocked.example.com".to_string()], - }; + let network = NetworkContext::new( + vec!["api.example.com".to_string(), "*.openai.com".to_string()], + vec!["blocked.example.com".to_string()], + ); let context = EnvironmentContext::new( Some(test_path_buf("/repo")), - fake_shell(), + fake_shell_name(), Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), Some(network), @@ -67,14 +69,14 @@ fn serialize_environment_context_with_network() { test_path_buf("/repo").display() ); - assert_eq!(context.serialize_to_xml(), expected); + assert_eq!(context.render(), expected); } #[test] fn serialize_read_only_environment_context() { let context = EnvironmentContext::new( /*cwd*/ None, - fake_shell(), + fake_shell_name(), Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), /*network*/ None, @@ -87,74 +89,14 @@ fn serialize_read_only_environment_context() { America/Los_Angeles "#; - assert_eq!(context.serialize_to_xml(), expected); -} - -#[test] -fn serialize_external_sandbox_environment_context() { - let context = EnvironmentContext::new( - /*cwd*/ None, - fake_shell(), - Some("2026-02-26".to_string()), - Some("America/Los_Angeles".to_string()), - /*network*/ None, - /*subagents*/ None, - ); - - let expected = r#" - bash - 2026-02-26 - America/Los_Angeles -"#; - - assert_eq!(context.serialize_to_xml(), expected); -} - -#[test] -fn serialize_external_sandbox_with_restricted_network_environment_context() { - let context = EnvironmentContext::new( - /*cwd*/ None, - fake_shell(), - Some("2026-02-26".to_string()), - Some("America/Los_Angeles".to_string()), - /*network*/ None, - /*subagents*/ None, - ); - - let expected = r#" - bash - 2026-02-26 - America/Los_Angeles -"#; - - assert_eq!(context.serialize_to_xml(), expected); -} - -#[test] -fn serialize_full_access_environment_context() { - let context = EnvironmentContext::new( - /*cwd*/ None, - fake_shell(), - Some("2026-02-26".to_string()), - Some("America/Los_Angeles".to_string()), - /*network*/ None, - /*subagents*/ None, - ); - - let expected = r#" - bash - 2026-02-26 - America/Los_Angeles -"#; - - assert_eq!(context.serialize_to_xml(), expected); + assert_eq!(context.render(), expected); } #[test] fn equals_except_shell_compares_cwd() { let context1 = EnvironmentContext::new( Some(PathBuf::from("/repo")), - fake_shell(), + fake_shell_name(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -162,7 +104,7 @@ fn equals_except_shell_compares_cwd() { ); let context2 = EnvironmentContext::new( Some(PathBuf::from("/repo")), - fake_shell(), + fake_shell_name(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -171,33 +113,11 @@ fn equals_except_shell_compares_cwd() { assert!(context1.equals_except_shell(&context2)); } -#[test] -fn equals_except_shell_ignores_sandbox_policy() { - let context1 = EnvironmentContext::new( - Some(PathBuf::from("/repo")), - fake_shell(), - /*current_date*/ None, - /*timezone*/ None, - /*network*/ None, - /*subagents*/ None, - ); - let context2 = EnvironmentContext::new( - Some(PathBuf::from("/repo")), - fake_shell(), - /*current_date*/ None, - /*timezone*/ None, - /*network*/ None, - /*subagents*/ None, - ); - - assert!(context1.equals_except_shell(&context2)); -} - #[test] fn equals_except_shell_compares_cwd_differences() { let context1 = EnvironmentContext::new( Some(PathBuf::from("/repo1")), - fake_shell(), + fake_shell_name(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -205,7 +125,7 @@ fn equals_except_shell_compares_cwd_differences() { ); let context2 = EnvironmentContext::new( Some(PathBuf::from("/repo2")), - fake_shell(), + fake_shell_name(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -219,11 +139,7 @@ fn equals_except_shell_compares_cwd_differences() { fn equals_except_shell_ignores_shell() { let context1 = EnvironmentContext::new( Some(PathBuf::from("/repo")), - Shell { - shell_type: ShellType::Bash, - shell_path: "/bin/bash".into(), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), - }, + "bash".to_string(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -231,11 +147,7 @@ fn equals_except_shell_ignores_shell() { ); let context2 = EnvironmentContext::new( Some(PathBuf::from("/repo")), - Shell { - shell_type: ShellType::Zsh, - shell_path: "/bin/zsh".into(), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), - }, + "zsh".to_string(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -249,7 +161,7 @@ fn equals_except_shell_ignores_shell() { fn serialize_environment_context_with_subagents() { let context = EnvironmentContext::new( Some(test_path_buf("/repo")), - fake_shell(), + fake_shell_name(), Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), /*network*/ None, @@ -270,5 +182,5 @@ fn serialize_environment_context_with_subagents() { test_path_buf("/repo").display() ); - assert_eq!(context.serialize_to_xml(), expected); + assert_eq!(context.render(), expected); } diff --git a/codex-rs/core/src/context/fragment.rs b/codex-rs/core/src/context/fragment.rs new file mode 100644 index 0000000000..e0e6d03303 --- /dev/null +++ b/codex-rs/core/src/context/fragment.rs @@ -0,0 +1,82 @@ +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; +use std::marker::PhantomData; + +/// Type-erased registration for a contextual user fragment. +/// +/// Implementations are used by context filtering code to recognize injected +/// fragments without constructing the concrete context payload. +pub(crate) trait FragmentRegistration: Sync { + fn matches_text(&self, text: &str) -> bool; +} + +pub(crate) struct FragmentRegistrationProxy { + _marker: PhantomData T>, +} + +impl FragmentRegistrationProxy { + pub(crate) const fn new() -> Self { + Self { + _marker: PhantomData, + } + } +} + +impl FragmentRegistration for FragmentRegistrationProxy { + fn matches_text(&self, text: &str) -> bool { + T::matches_text(text) + } +} + +/// Context payload that is injected as a user-authored message fragment. +/// +/// Implementations own the response role, start/end markers used to recognize +/// the fragment, and provide the fragment body appended directly after the +/// start marker. The default helpers wrap that body and convert it into the +/// response item shape expected by model input assembly. +pub(crate) trait ContextualUserFragment { + const ROLE: &'static str; + const START_MARKER: &'static str; + const END_MARKER: &'static str; + + fn body(&self) -> String; + + fn matches_text(text: &str) -> bool + where + Self: Sized, + { + let trimmed = text.trim_start(); + let starts_with_marker = trimmed + .get(..Self::START_MARKER.len()) + .is_some_and(|candidate| candidate.eq_ignore_ascii_case(Self::START_MARKER)); + let trimmed = trimmed.trim_end(); + let ends_with_marker = trimmed + .get(trimmed.len().saturating_sub(Self::END_MARKER.len())..) + .is_some_and(|candidate| candidate.eq_ignore_ascii_case(Self::END_MARKER)); + starts_with_marker && ends_with_marker + } + + fn render(&self) -> String { + format!( + "{}{}\n{}", + Self::START_MARKER, + self.body(), + Self::END_MARKER + ) + } + + fn into(self) -> ResponseItem + where + Self: Sized, + { + ResponseItem::Message { + id: None, + role: Self::ROLE.to_string(), + content: vec![ContentItem::InputText { + text: self.render(), + }], + end_turn: None, + phase: None, + } + } +} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs new file mode 100644 index 0000000000..94c5849f9d --- /dev/null +++ b/codex-rs/core/src/context/mod.rs @@ -0,0 +1,21 @@ +mod contextual_user_message; +mod environment_context; +mod fragment; +mod skill_instructions; +mod subagent_notification; +mod turn_aborted; +mod user_instructions; +mod user_shell_command; + +pub(crate) use contextual_user_message::is_contextual_user_fragment; +pub(crate) use contextual_user_message::is_memory_excluded_contextual_user_fragment; +pub(crate) use contextual_user_message::parse_visible_hook_prompt_message; +pub(crate) use environment_context::EnvironmentContext; +pub(crate) use fragment::ContextualUserFragment; +pub(crate) use fragment::FragmentRegistration; +pub(crate) use fragment::FragmentRegistrationProxy; +pub(crate) use skill_instructions::SkillInstructions; +pub(crate) use subagent_notification::SubagentNotification; +pub(crate) use turn_aborted::TurnAborted; +pub(crate) use user_instructions::UserInstructions; +pub(crate) use user_shell_command::UserShellCommand; diff --git a/codex-rs/core/src/context/skill_instructions.rs b/codex-rs/core/src/context/skill_instructions.rs new file mode 100644 index 0000000000..90271b807e --- /dev/null +++ b/codex-rs/core/src/context/skill_instructions.rs @@ -0,0 +1,33 @@ +use codex_core_skills::injection::SkillInjection; + +use super::ContextualUserFragment; + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct SkillInstructions { + pub(crate) name: String, + pub(crate) path: String, + pub(crate) contents: String, +} + +impl From<&SkillInjection> for SkillInstructions { + fn from(skill: &SkillInjection) -> Self { + Self { + name: skill.name.clone(), + path: skill.path.clone(), + contents: skill.contents.clone(), + } + } +} + +impl ContextualUserFragment for SkillInstructions { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn body(&self) -> String { + format!( + "\n{}\n{}\n{}", + self.name, self.path, self.contents + ) + } +} diff --git a/codex-rs/core/src/context/subagent_notification.rs b/codex-rs/core/src/context/subagent_notification.rs new file mode 100644 index 0000000000..16f7fb4747 --- /dev/null +++ b/codex-rs/core/src/context/subagent_notification.rs @@ -0,0 +1,34 @@ +use codex_protocol::protocol::AgentStatus; + +use super::ContextualUserFragment; + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct SubagentNotification { + pub(crate) agent_reference: String, + pub(crate) status: AgentStatus, +} + +impl SubagentNotification { + pub(crate) fn new(agent_reference: impl Into, status: AgentStatus) -> Self { + Self { + agent_reference: agent_reference.into(), + status, + } + } +} + +impl ContextualUserFragment for SubagentNotification { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn body(&self) -> String { + format!( + "\n{}", + serde_json::json!({ + "agent_path": &self.agent_reference, + "status": &self.status, + }) + ) + } +} diff --git a/codex-rs/core/src/context/turn_aborted.rs b/codex-rs/core/src/context/turn_aborted.rs new file mode 100644 index 0000000000..69620c0042 --- /dev/null +++ b/codex-rs/core/src/context/turn_aborted.rs @@ -0,0 +1,26 @@ +use super::ContextualUserFragment; + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct TurnAborted { + pub(crate) guidance: String, +} + +impl TurnAborted { + pub(crate) const INTERRUPTED_GUIDANCE: &'static str = "The user interrupted the previous turn on purpose. Any running unified exec processes may still be running in the background. If any tools/commands were aborted, they may have partially executed."; + + pub(crate) fn new(guidance: impl Into) -> Self { + Self { + guidance: guidance.into(), + } + } +} + +impl ContextualUserFragment for TurnAborted { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn body(&self) -> String { + format!("\n{}", self.guidance) + } +} diff --git a/codex-rs/core/src/context/user_instructions.rs b/codex-rs/core/src/context/user_instructions.rs new file mode 100644 index 0000000000..97efc4f45c --- /dev/null +++ b/codex-rs/core/src/context/user_instructions.rs @@ -0,0 +1,17 @@ +use super::ContextualUserFragment; + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct UserInstructions { + pub(crate) directory: String, + pub(crate) text: String, +} + +impl ContextualUserFragment for UserInstructions { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = "# AGENTS.md instructions for "; + const END_MARKER: &'static str = ""; + + fn body(&self) -> String { + format!("{}\n\n\n{}", self.directory, self.text) + } +} diff --git a/codex-rs/core/src/context/user_shell_command.rs b/codex-rs/core/src/context/user_shell_command.rs new file mode 100644 index 0000000000..8bbbb952d8 --- /dev/null +++ b/codex-rs/core/src/context/user_shell_command.rs @@ -0,0 +1,40 @@ +use std::time::Duration; + +use super::ContextualUserFragment; + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct UserShellCommand { + pub(crate) command: String, + pub(crate) exit_code: i32, + pub(crate) duration_seconds: f64, + pub(crate) output: String, +} + +impl UserShellCommand { + pub(crate) fn new( + command: impl Into, + exit_code: i32, + duration: Duration, + output: impl Into, + ) -> Self { + Self { + command: command.into(), + exit_code, + duration_seconds: duration.as_secs_f64(), + output: output.into(), + } + } +} + +impl ContextualUserFragment for UserShellCommand { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn body(&self) -> String { + format!( + "\n\n{}\n\n\nExit code: {}\nDuration: {:.4} seconds\nOutput:\n{}\n", + self.command, self.exit_code, self.duration_seconds, self.output, + ) + } +} diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index 5ced481530..a3ccf49f83 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -1,4 +1,5 @@ -use crate::environment_context::EnvironmentContext; +use crate::context::ContextualUserFragment; +use crate::context::EnvironmentContext; use crate::session::PreviousTurnSettings; use crate::session::turn_context::TurnContext; use crate::shell::Shell; @@ -21,14 +22,14 @@ fn build_environment_update_item( } let prev = previous?; - let prev_context = EnvironmentContext::from_turn_context_item(prev, shell); + let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string()); let next_context = EnvironmentContext::from_turn_context(next, shell); if prev_context.equals_except_shell(&next_context) { return None; } - Some(ResponseItem::from( - EnvironmentContext::diff_from_turn_context_item(prev, next, shell), + Some(ContextualUserFragment::into( + EnvironmentContext::diff_from_turn_context_item(prev, &next_context), )) } diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 21e13f6c15..e7c79e6dd2 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -20,8 +20,8 @@ use codex_protocol::user_input::UserInput; use tracing::warn; use uuid::Uuid; -use crate::contextual_user_message::is_contextual_user_fragment; -use crate::contextual_user_message::parse_visible_hook_prompt_message; +use crate::context::is_contextual_user_fragment; +use crate::context::parse_visible_hook_prompt_message; use crate::web_search::web_search_action_detail; const CONTEXTUAL_DEVELOPER_PREFIXES: &[&str] = &[ diff --git a/codex-rs/core/src/instructions/mod.rs b/codex-rs/core/src/instructions/mod.rs deleted file mode 100644 index a1f77ba5e8..0000000000 --- a/codex-rs/core/src/instructions/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub(crate) use codex_instructions::UserInstructions; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 1809789a8e..fc96aceaf9 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -27,9 +27,8 @@ mod commit_attribution; pub mod config; pub mod config_loader; pub mod connectors; +mod context; mod context_manager; -mod contextual_user_message; -mod environment_context; pub mod exec; pub mod exec_env; mod exec_policy; @@ -41,7 +40,6 @@ mod git_info_tests; mod guardian; mod hook_runtime; mod installation_id; -pub(crate) mod instructions; pub(crate) mod landlock; pub use landlock::spawn_command_under_linux_sandbox; pub(crate) mod mcp; diff --git a/codex-rs/core/src/memories/phase1.rs b/codex-rs/core/src/memories/phase1.rs index 9d46374f68..4f19caa5c5 100644 --- a/codex-rs/core/src/memories/phase1.rs +++ b/codex-rs/core/src/memories/phase1.rs @@ -1,7 +1,7 @@ use crate::Prompt; use crate::RolloutRecorder; use crate::config::Config; -use crate::contextual_user_message::is_memory_excluded_contextual_user_fragment; +use crate::context::is_memory_excluded_contextual_user_fragment; use crate::memories::metrics; use crate::memories::phase_one; use crate::memories::phase_one::PRUNE_BATCH_SIZE; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index f1fea3517a..3ea826c491 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -19,6 +19,7 @@ use crate::commit_attribution::commit_message_trailer_instruction; use crate::compact; use crate::config::ManagedFeatures; use crate::connectors; +use crate::context::ContextualUserFragment; use crate::default_skill_metadata_budget; use crate::exec_policy::ExecPolicyManager; use crate::installation_id::resolve_installation_id; @@ -153,7 +154,6 @@ use crate::config::StartedNetworkProxy; use crate::config::resolve_web_search_mode_for_turn; use crate::context_manager::ContextManager; use crate::context_manager::TotalTokenUsageBreakdown; -use crate::environment_context::EnvironmentContext; use crate::thread_rollout_truncation::initial_history_has_prior_user_turns; use codex_config::CONFIG_TOML_FILE; use codex_config::types::McpServerConfig; @@ -249,9 +249,9 @@ use crate::SkillLoadOutcome; use crate::SkillMetadata; use crate::SkillsManager; use crate::agents_md::AgentsMdManager; +use crate::context::UserInstructions; use crate::exec_policy::ExecPolicyUpdateError; use crate::guardian::GuardianReviewSessionManager; -use crate::instructions::UserInstructions; use crate::mcp::McpManager; use crate::memories; use crate::network_policy_decision::execpolicy_network_rule_amendment; @@ -2487,7 +2487,7 @@ impl Session { text: user_instructions.to_string(), directory: turn_context.cwd.to_string_lossy().into_owned(), } - .serialize_to_text(), + .render(), ); } if turn_context.config.include_environment_context { @@ -2497,9 +2497,9 @@ impl Session { .format_environment_context_subagents(self.conversation_id) .await; contextual_user_sections.push( - EnvironmentContext::from_turn_context(turn_context, shell.as_ref()) + crate::context::EnvironmentContext::from_turn_context(turn_context, shell.as_ref()) .with_subagents(subagents) - .serialize_to_xml(), + .render(), ); } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index f901e5bee4..18cefba934 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -11,6 +11,8 @@ use crate::config_loader::NetworkDomainPermissionsToml; use crate::config_loader::RequirementSource; use crate::config_loader::Sourced; use crate::config_loader::project_trust_key; +use crate::context::ContextualUserFragment; +use crate::context::TurnAborted; use crate::exec::ExecCapturePolicy; use crate::function_tool::FunctionCallError; use crate::shell::default_user_shell; @@ -6349,7 +6351,7 @@ async fn abort_review_task_emits_exited_then_aborted_and_records_history() { let ContentItem::InputText { text } = content_item else { return false; }; - text.contains(crate::contextual_user_message::TURN_ABORTED_OPEN_TAG) + TurnAborted::matches_text(text) }) }), "expected a model-visible turn aborted marker in history after interrupt" diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index efbcca6716..f613802244 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -16,6 +16,7 @@ use crate::compact::run_inline_auto_compact_task; use crate::compact::should_use_remote_compact_task; use crate::compact_remote::run_inline_remote_auto_compact_task; use crate::connectors; +use crate::context::ContextualUserFragment; use crate::feedback_tags; use crate::hook_runtime::PendingInputHookDisposition; use crate::hook_runtime::emit_hook_completed_events; @@ -241,7 +242,7 @@ pub(crate) async fn run_turn( turn_context.sub_id.clone(), ); let SkillInjections { - items: skill_items, + items: skill_injections, warnings: skill_warnings, } = build_skill_injections( &mentioned_skills, @@ -257,6 +258,11 @@ pub(crate) async fn run_turn( .await; } + let skill_items: Vec = skill_injections + .iter() + .map(|skill| ContextualUserFragment::into(crate::context::SkillInstructions::from(skill))) + .collect(); + let plugin_items = build_plugin_injections(&mentioned_plugins, &mcp_tools, &available_connectors); let mentioned_plugin_metadata = mentioned_plugins diff --git a/codex-rs/core/src/session_prefix.rs b/codex-rs/core/src/session_prefix.rs index 42f213a1d3..b50d1f9070 100644 --- a/codex-rs/core/src/session_prefix.rs +++ b/codex-rs/core/src/session_prefix.rs @@ -1,20 +1,17 @@ use codex_protocol::protocol::AgentStatus; -/// Helpers for model-visible session state markers that are stored in user-role -/// messages but are not user intent. -use crate::contextual_user_message::SUBAGENT_NOTIFICATION_FRAGMENT; +use crate::context::ContextualUserFragment; +use crate::context::SubagentNotification; + +// Helpers for model-visible session state markers that are stored in user-role +// messages but are not user intent. // TODO(jif) unify with structured schema pub(crate) fn format_subagent_notification_message( agent_reference: &str, status: &AgentStatus, ) -> String { - let payload_json = serde_json::json!({ - "agent_path": agent_reference, - "status": status, - }) - .to_string(); - SUBAGENT_NOTIFICATION_FRAGMENT.wrap(payload_json) + SubagentNotification::new(agent_reference, status.clone()).render() } pub(crate) fn format_subagent_context_line( diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index e7030d1d94..9c19087204 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -19,8 +19,7 @@ use tracing::info_span; use tracing::trace; use tracing::warn; -use crate::contextual_user_message::TURN_ABORTED_CLOSE_TAG; -use crate::contextual_user_message::TURN_ABORTED_OPEN_TAG; +use crate::context::ContextualUserFragment; use crate::hook_runtime::PendingInputHookDisposition; use crate::hook_runtime::inspect_pending_input; use crate::hook_runtime::record_additional_contexts; @@ -39,7 +38,6 @@ use codex_otel::TURN_MEMORY_METRIC; use codex_otel::TURN_NETWORK_PROXY_METRIC; use codex_otel::TURN_TOKEN_USAGE_METRIC; use codex_otel::TURN_TOOL_CALL_METRIC; -use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::EventMsg; @@ -62,22 +60,13 @@ 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 may still be running in the background. If any tools/commands were aborted, they may have partially executed."; /// Shared model-visible marker used by both the real interrupt path and /// interrupted fork snapshots. pub(crate) fn interrupted_turn_history_marker() -> ResponseItem { - ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: format!( - "{TURN_ABORTED_OPEN_TAG}\n{TURN_ABORTED_INTERRUPTED_GUIDANCE}\n{TURN_ABORTED_CLOSE_TAG}" - ), - }], - end_turn: None, - phase: None, - } + ContextualUserFragment::into(crate::context::TurnAborted::new( + crate::context::TurnAborted::INTERRUPTED_GUIDANCE, + )) } fn emit_turn_network_proxy_metric( diff --git a/codex-rs/core/src/user_shell_command.rs b/codex-rs/core/src/user_shell_command.rs index e244be0d2a..cf034faca0 100644 --- a/codex-rs/core/src/user_shell_command.rs +++ b/codex-rs/core/src/user_shell_command.rs @@ -1,45 +1,27 @@ -use std::time::Duration; - use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::models::ResponseItem; -use crate::contextual_user_message::USER_SHELL_COMMAND_FRAGMENT; +use crate::context::ContextualUserFragment; +use crate::context::UserShellCommand; use crate::session::turn_context::TurnContext; 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") -} - -fn format_user_shell_command_body( +fn user_shell_command_fragment( command: &str, exec_output: &ExecToolCallOutput, turn_context: &TurnContext, -) -> String { - 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()); - sections.join("\n") +) -> UserShellCommand { + let output = format_exec_output_str(exec_output, turn_context.truncation_policy); + UserShellCommand::new(command, exec_output.exit_code, exec_output.duration, output) } +#[cfg(test)] pub fn format_user_shell_command_record( command: &str, exec_output: &ExecToolCallOutput, turn_context: &TurnContext, ) -> String { - let body = format_user_shell_command_body(command, exec_output, turn_context); - USER_SHELL_COMMAND_FRAGMENT.wrap(body) + user_shell_command_fragment(command, exec_output, turn_context).render() } pub fn user_shell_command_record_item( @@ -47,7 +29,7 @@ pub fn user_shell_command_record_item( exec_output: &ExecToolCallOutput, turn_context: &TurnContext, ) -> ResponseItem { - USER_SHELL_COMMAND_FRAGMENT.into_message(format_user_shell_command_record( + ContextualUserFragment::into(user_shell_command_fragment( command, exec_output, turn_context, diff --git a/codex-rs/core/src/user_shell_command_tests.rs b/codex-rs/core/src/user_shell_command_tests.rs index 7e68ca5fee..996d66919a 100644 --- a/codex-rs/core/src/user_shell_command_tests.rs +++ b/codex-rs/core/src/user_shell_command_tests.rs @@ -1,16 +1,18 @@ use super::*; +use crate::context::ContextualUserFragment; +use crate::context::UserShellCommand; use crate::session::tests::make_session_and_context; use codex_protocol::exec_output::StreamOutput; use codex_protocol::models::ContentItem; use pretty_assertions::assert_eq; +use std::time::Duration; #[test] fn detects_user_shell_command_text_variants() { - assert!( - USER_SHELL_COMMAND_FRAGMENT - .matches_text("\necho hi\n") - ); - assert!(!USER_SHELL_COMMAND_FRAGMENT.matches_text("echo hi")); + assert!(UserShellCommand::matches_text( + "\necho hi\n" + )); + assert!(!UserShellCommand::matches_text("echo hi")); } #[tokio::test] diff --git a/codex-rs/instructions/BUILD.bazel b/codex-rs/instructions/BUILD.bazel deleted file mode 100644 index d3cdbd19d1..0000000000 --- a/codex-rs/instructions/BUILD.bazel +++ /dev/null @@ -1,16 +0,0 @@ -load("//:defs.bzl", "codex_rust_crate") - -codex_rust_crate( - name = "instructions", - crate_name = "codex_instructions", - compile_data = glob( - include = ["**"], - exclude = [ - "BUILD.bazel", - "Cargo.toml", - ], - allow_empty = True, - ) + [ - "//codex-rs:node-version.txt", - ], -) diff --git a/codex-rs/instructions/Cargo.toml b/codex-rs/instructions/Cargo.toml deleted file mode 100644 index cdaa3d7368..0000000000 --- a/codex-rs/instructions/Cargo.toml +++ /dev/null @@ -1,20 +0,0 @@ -[package] -edition.workspace = true -license.workspace = true -name = "codex-instructions" -version.workspace = true - -[lib] -doctest = false -name = "codex_instructions" -path = "src/lib.rs" - -[lints] -workspace = true - -[dependencies] -codex-protocol = { workspace = true } -serde = { workspace = true, features = ["derive"] } - -[dev-dependencies] -pretty_assertions = { workspace = true } diff --git a/codex-rs/instructions/src/fragment.rs b/codex-rs/instructions/src/fragment.rs deleted file mode 100644 index 590dac379b..0000000000 --- a/codex-rs/instructions/src/fragment.rs +++ /dev/null @@ -1,61 +0,0 @@ -use codex_protocol::models::ContentItem; -use codex_protocol::models::ResponseItem; - -pub(crate) const AGENTS_MD_START_MARKER: &str = "# AGENTS.md instructions for "; -pub(crate) const AGENTS_MD_END_MARKER: &str = ""; -pub(crate) const SKILL_OPEN_TAG: &str = ""; -pub(crate) const SKILL_CLOSE_TAG: &str = ""; - -#[derive(Clone, Copy)] -pub struct ContextualUserFragmentDefinition { - start_marker: &'static str, - end_marker: &'static str, -} - -impl ContextualUserFragmentDefinition { - pub const fn new(start_marker: &'static str, end_marker: &'static str) -> Self { - Self { - start_marker, - end_marker, - } - } - - pub fn matches_text(&self, text: &str) -> bool { - let trimmed = text.trim_start(); - let starts_with_marker = trimmed - .get(..self.start_marker.len()) - .is_some_and(|candidate| candidate.eq_ignore_ascii_case(self.start_marker)); - let trimmed = trimmed.trim_end(); - let ends_with_marker = trimmed - .get(trimmed.len().saturating_sub(self.end_marker.len())..) - .is_some_and(|candidate| candidate.eq_ignore_ascii_case(self.end_marker)); - starts_with_marker && ends_with_marker - } - - pub const fn start_marker(&self) -> &'static str { - self.start_marker - } - - pub const fn end_marker(&self) -> &'static str { - self.end_marker - } - - pub fn wrap(&self, body: String) -> String { - format!("{}\n{}\n{}", self.start_marker, body, self.end_marker) - } - - pub fn into_message(self, text: String) -> ResponseItem { - ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { text }], - end_turn: None, - phase: None, - } - } -} - -pub const AGENTS_MD_FRAGMENT: ContextualUserFragmentDefinition = - ContextualUserFragmentDefinition::new(AGENTS_MD_START_MARKER, AGENTS_MD_END_MARKER); -pub const SKILL_FRAGMENT: ContextualUserFragmentDefinition = - ContextualUserFragmentDefinition::new(SKILL_OPEN_TAG, SKILL_CLOSE_TAG); diff --git a/codex-rs/instructions/src/lib.rs b/codex-rs/instructions/src/lib.rs deleted file mode 100644 index 1b1f81550c..0000000000 --- a/codex-rs/instructions/src/lib.rs +++ /dev/null @@ -1,11 +0,0 @@ -//! User and skill instruction payloads and contextual user fragment markers for Codex prompts. - -mod fragment; -mod user_instructions; - -pub use fragment::AGENTS_MD_FRAGMENT; -pub use fragment::ContextualUserFragmentDefinition; -pub use fragment::SKILL_FRAGMENT; -pub use user_instructions::SkillInstructions; -pub use user_instructions::USER_INSTRUCTIONS_PREFIX; -pub use user_instructions::UserInstructions; diff --git a/codex-rs/instructions/src/user_instructions.rs b/codex-rs/instructions/src/user_instructions.rs deleted file mode 100644 index 4fd266e766..0000000000 --- a/codex-rs/instructions/src/user_instructions.rs +++ /dev/null @@ -1,56 +0,0 @@ -use serde::Deserialize; -use serde::Serialize; - -use codex_protocol::models::ResponseItem; - -use crate::fragment::AGENTS_MD_FRAGMENT; -use crate::fragment::AGENTS_MD_START_MARKER; -use crate::fragment::SKILL_FRAGMENT; - -pub const USER_INSTRUCTIONS_PREFIX: &str = AGENTS_MD_START_MARKER; - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(rename = "user_instructions", rename_all = "snake_case")] -pub struct UserInstructions { - pub directory: String, - pub text: String, -} - -impl UserInstructions { - pub fn serialize_to_text(&self) -> String { - format!( - "{prefix}{directory}\n\n\n{contents}\n{suffix}", - prefix = AGENTS_MD_FRAGMENT.start_marker(), - directory = self.directory, - contents = self.text, - suffix = AGENTS_MD_FRAGMENT.end_marker(), - ) - } -} - -impl From for ResponseItem { - fn from(ui: UserInstructions) -> Self { - AGENTS_MD_FRAGMENT.into_message(ui.serialize_to_text()) - } -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(rename = "skill_instructions", rename_all = "snake_case")] -pub struct SkillInstructions { - pub name: String, - pub path: String, - pub contents: String, -} - -impl From for ResponseItem { - fn from(si: SkillInstructions) -> Self { - SKILL_FRAGMENT.into_message(SKILL_FRAGMENT.wrap(format!( - "{}\n{}\n{}", - si.name, si.path, si.contents - ))) - } -} - -#[cfg(test)] -#[path = "user_instructions_tests.rs"] -mod tests; diff --git a/codex-rs/instructions/src/user_instructions_tests.rs b/codex-rs/instructions/src/user_instructions_tests.rs deleted file mode 100644 index 75f35d11b0..0000000000 --- a/codex-rs/instructions/src/user_instructions_tests.rs +++ /dev/null @@ -1,72 +0,0 @@ -use super::*; -use codex_protocol::models::ContentItem; -use codex_protocol::models::ResponseItem; -use pretty_assertions::assert_eq; - -use crate::fragment::AGENTS_MD_FRAGMENT; -use crate::fragment::SKILL_FRAGMENT; - -#[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")); -}