diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 192e7b19da1c..2ce64064432f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1978,6 +1978,7 @@ dependencies = [ "codex-git-utils", "codex-goal-extension", "codex-guardian", + "codex-home", "codex-hooks", "codex-image-generation-extension", "codex-login", @@ -2598,6 +2599,7 @@ dependencies = [ "codex-features", "codex-feedback", "codex-git-utils", + "codex-home", "codex-hooks", "codex-image-generation-extension", "codex-install-context", @@ -3266,6 +3268,7 @@ dependencies = [ "codex-core", "codex-exec-server", "codex-extension-api", + "codex-home", "codex-login", "codex-protocol", "codex-shell-command", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 6f384eff723c..9e9a4f4699b4 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -37,8 +37,8 @@ members = [ "core-api", "core-plugins", "core-skills", - "home", "hooks", + "home", "secrets", "exec", "file-system", @@ -170,7 +170,6 @@ codex-execpolicy = { path = "execpolicy" } codex-extension-api = { path = "ext/extension-api" } codex-goal-extension = { path = "ext/goal" } codex-guardian = { path = "ext/guardian" } -codex-home = { path = "home" } codex-image-generation-extension = { path = "ext/image-generation" } codex-external-agent-migration = { path = "external-agent-migration" } codex-external-agent-sessions = { path = "external-agent-sessions" } @@ -182,6 +181,7 @@ codex-file-search = { path = "file-search" } codex-file-watcher = { path = "file-watcher" } codex-git-utils = { path = "git-utils" } codex-hooks = { path = "hooks" } +codex-home = { path = "home" } codex-keyring-store = { path = "keyring-store" } codex-linux-sandbox = { path = "linux-sandbox" } codex-lmstudio = { path = "lmstudio" } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 4bdc430a1a90..eaa35ed4c91f 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -46,6 +46,7 @@ codex-guardian = { workspace = true } codex-git-utils = { workspace = true } codex-file-watcher = { workspace = true } codex-hooks = { workspace = true } +codex-home = { workspace = true } codex-otel = { workspace = true } codex-plugin = { workspace = true } codex-shell-command = { workspace = true } diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index 53f19997cd1f..4eda1fb8d51c 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -14,12 +14,14 @@ use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistry; use codex_extension_api::ExtensionRegistryBuilder; use codex_goal_extension::GoalService; +use codex_home::CodexHomeInstructionsContributor; use codex_login::AuthManager; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_rollout::state_db::StateDbHandle; +use codex_utils_absolute_path::AbsolutePathBuf; use crate::outgoing_message::OutgoingMessageSender; use crate::thread_state::ThreadListenerCommand; @@ -32,11 +34,15 @@ pub(crate) fn thread_extensions( state_db: Option, thread_manager: Weak, goal_service: Arc, + codex_home: AbsolutePathBuf, ) -> Arc> where S: AgentSpawner + 'static, { let mut builder = ExtensionRegistryBuilder::::with_event_sink(event_sink); + builder.global_instructions_contributor(Arc::new(CodexHomeInstructionsContributor::new( + codex_home, + ))); if let Some(state_db) = state_db { codex_goal_extension::install_with_backend( &mut builder, diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index 8c72533ba52a..64d66fad9b7b 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -194,6 +194,7 @@ mod tests { Some(state_db.clone()), thread_manager.clone(), Arc::new(codex_goal_extension::GoalService::new()), + good_config.codex_home.clone(), ), /*analytics_events_client*/ None, Arc::clone(&thread_store), diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index b7e0ff258d37..c6482a942f37 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -320,6 +320,7 @@ impl MessageProcessor { state_db.clone(), thread_manager.clone(), Arc::clone(&goal_service), + config.codex_home.clone(), ), Some(analytics_events_client.clone()), Arc::clone(&thread_store), diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 6366c6fbcaa5..ca115e9a99d4 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -30,6 +30,7 @@ use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::config_types::TrustLevel; use codex_protocol::openai_models::ReasoningEffort; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -400,11 +401,13 @@ async fn thread_start_response_excludes_empty_project_instruction_source() -> Re } #[tokio::test] -async fn thread_start_without_selected_environment_excludes_instruction_sources() -> Result<()> { +async fn thread_start_without_selected_environment_includes_global_instruction_source() -> Result<()> +{ let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?; - std::fs::write(codex_home.path().join("AGENTS.md"), "global instructions")?; + let global_agents_path = codex_home.path().join("AGENTS.md"); + std::fs::write(&global_agents_path, "global instructions")?; let workspace = TempDir::new()?; std::fs::write(workspace.path().join("AGENTS.md"), "project instructions")?; @@ -428,7 +431,12 @@ async fn thread_start_without_selected_environment_excludes_instruction_sources( .. } = to_response::(response)?; - assert!(instruction_sources.is_empty()); + assert_eq!( + instruction_sources, + vec![AbsolutePathBuf::try_from(std::fs::canonicalize( + global_agents_path + )?)?] + ); Ok(()) } diff --git a/codex-rs/core-api/Cargo.toml b/codex-rs/core-api/Cargo.toml index 3812b56ecb69..f4c89e5640cc 100644 --- a/codex-rs/core-api/Cargo.toml +++ b/codex-rs/core-api/Cargo.toml @@ -20,9 +20,9 @@ codex-analytics = { workspace = true } codex-config = { workspace = true } codex-core = { workspace = true } codex-extension-api = { workspace = true } +codex-home = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } -codex-home = { workspace = true } codex-login = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index ed3ac2bfe86f..304390c633f2 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -26,7 +26,6 @@ pub use codex_config::types::TuiPetAnchor; pub use codex_config::types::UriBasedFileOpener; pub use codex_core::CodexThread; pub use codex_core::ForkSnapshot; -pub use codex_core::LoadedAgentsMd; pub use codex_core::McpManager; pub use codex_core::NewThread; pub use codex_core::StartThreadOptions; diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 352e8051e4d6..633ba99ad74b 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -48,6 +48,7 @@ codex-shell-command = { workspace = true } codex-execpolicy = { workspace = true } codex-git-utils = { workspace = true } codex-hooks = { workspace = true } +codex-home = { workspace = true } codex-install-context = { workspace = true } codex-network-proxy = { workspace = true } codex-otel = { workspace = true } diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 8905ad48c83d..0d140650642d 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -55,36 +55,6 @@ impl<'a> AgentsMdManager<'a> { Self { config } } - pub(crate) async fn load_global_instructions( - fs: &dyn ExecutorFileSystem, - codex_dir: Option<&AbsolutePathBuf>, - startup_warnings: &mut Vec, - ) -> Option { - let base = codex_dir?; - for candidate in [LOCAL_AGENTS_MD_FILENAME, DEFAULT_AGENTS_MD_FILENAME] { - let path = base.join(candidate); - let data = match fs.read_file(&path, /*sandbox*/ None).await { - Ok(data) => data, - Err(err) if err.kind() == io::ErrorKind::NotFound => continue, - Err(err) if err.kind() == io::ErrorKind::IsADirectory => continue, - Err(err) => { - startup_warnings.push(format!( - "Failed to read global AGENTS.md instructions from `{}`: {err}", - path.display() - )); - continue; - } - }; - warn_invalid_utf8(&path, &data, "Global", startup_warnings); - let contents = String::from_utf8_lossy(&data); - let trimmed = contents.trim(); - if !trimmed.is_empty() { - return Some(LoadedAgentsMd::new_user(trimmed.to_string(), path)); - } - } - None - } - /// Combines global instructions and project AGENTS.md content into a /// single model-visible instruction snapshot. pub(crate) async fn user_instructions( @@ -321,27 +291,25 @@ impl<'a> AgentsMdManager<'a> { /// Model-visible instructions loaded from AGENTS.md files and internal /// guidance. #[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct LoadedAgentsMd { +pub(crate) struct LoadedAgentsMd { /// Ordered instructions and their provenance. entries: Vec, } impl LoadedAgentsMd { - pub fn new_user(contents: String, path: AbsolutePathBuf) -> Self { - if contents.trim().is_empty() { - return Self::default(); - } - let mut loaded = Self { - entries: vec![InstructionEntry { + #[cfg(test)] + pub(crate) fn new_user(contents: String, path: AbsolutePathBuf) -> Self { + Self::from_global(codex_extension_api::GlobalInstructions { + instructions: vec![codex_extension_api::GlobalInstruction::new( contents, - provenance: InstructionProvenance::Global(Some(path)), - }], - }; - loaded.enforce_model_context_limit(); - loaded + Some(path), + )], + warnings: Vec::new(), + }) } - pub fn from_text_for_testing(contents: impl Into) -> Self { + #[cfg(test)] + pub(crate) fn from_text_for_testing(contents: impl Into) -> Self { let contents = contents.into(); if contents.trim().is_empty() { return Self::default(); @@ -354,6 +322,22 @@ impl LoadedAgentsMd { } } + pub(crate) fn from_global(instructions: codex_extension_api::GlobalInstructions) -> Self { + let mut loaded = Self { + entries: instructions + .instructions + .into_iter() + .filter(|instruction| !instruction.contents.trim().is_empty()) + .map(|instruction| InstructionEntry { + contents: instruction.contents, + provenance: InstructionProvenance::Global(instruction.source), + }) + .collect(), + }; + loaded.enforce_model_context_limit(); + loaded + } + pub(crate) fn from_snapshot(snapshot: UserInstructionsSnapshot) -> Self { let mut loaded = Self { entries: snapshot @@ -403,14 +387,14 @@ impl LoadedAgentsMd { } } - pub(crate) fn replace_global(&mut self, instructions: LoadedAgentsMd) { + pub(crate) fn replace_global(&mut self, instructions: codex_extension_api::GlobalInstructions) { let first_non_global = self .entries .iter() .position(|entry| !matches!(entry.provenance, InstructionProvenance::Global(_))) .unwrap_or(self.entries.len()); self.entries - .splice(..first_non_global, instructions.entries); + .splice(..first_non_global, Self::from_global(instructions).entries); self.enforce_model_context_limit(); } @@ -421,7 +405,7 @@ impl LoadedAgentsMd { } /// Returns the concatenated model-visible instruction text. - pub fn text(&self) -> String { + pub(crate) fn text(&self) -> String { let mut output = String::new(); let mut previous_provenance: Option<&InstructionProvenance> = None; for entry in &self.entries { @@ -445,7 +429,7 @@ impl LoadedAgentsMd { } /// Returns the AGENTS.md files that supplied instruction entries. - pub fn sources(&self) -> impl Iterator { + pub(crate) fn sources(&self) -> impl Iterator { self.entries .iter() .filter_map(|entry| entry.provenance.path()) diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index 275c5ae015ea..b26fc1efb985 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -188,15 +188,15 @@ fn loaded_instructions_with_only_empty_or_whitespace_entries_are_empty() { } #[test] -fn configured_and_persisted_instructions_are_bounded_for_model_context() { +fn contributor_and_persisted_instructions_are_bounded_for_model_context() { let oversized = "x".repeat(approx_bytes_for_tokens(MAX_USER_INSTRUCTIONS_TOKENS + 100)); - let mut global = LoadedAgentsMd { - entries: vec![InstructionEntry { - contents: oversized.clone(), - provenance: InstructionProvenance::Global(None), - }], - }; - global.enforce_model_context_limit(); + let global = LoadedAgentsMd::from_global(codex_extension_api::GlobalInstructions { + instructions: vec![codex_extension_api::GlobalInstruction::new( + oversized.clone(), + /*source*/ None, + )], + warnings: Vec::new(), + }); let restored = LoadedAgentsMd::from_snapshot(UserInstructionsSnapshot { instructions: vec![InstructionSnapshot { contents: oversized, @@ -227,7 +227,13 @@ fn replacing_global_instructions_preserves_project_entries_within_the_bound() { provenance: InstructionProvenance::Project(Some(project_source.clone())), }); - loaded.replace_global(LoadedAgentsMd::from_text_for_testing("new global")); + loaded.replace_global(codex_extension_api::GlobalInstructions { + instructions: vec![codex_extension_api::GlobalInstruction::new( + "new global", + /*source*/ None, + )], + warnings: Vec::new(), + }); assert_eq!( loaded, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 3a3b490d697c..22f3f9ba29b0 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1,5 +1,3 @@ -use crate::agents_md::DEFAULT_AGENTS_MD_FILENAME; -use crate::agents_md::LOCAL_AGENTS_MD_FILENAME; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config::edit::apply_blocking; @@ -205,61 +203,6 @@ async fn load_config_normalizes_relative_cwd_override() -> std::io::Result<()> { Ok(()) } -#[tokio::test] -async fn load_config_loads_global_agents_instructions() -> std::io::Result<()> { - let codex_home = tempdir()?; - let global_agents_path = codex_home.abs().join(DEFAULT_AGENTS_MD_FILENAME); - std::fs::write(&global_agents_path, "\n global instructions \n")?; - - let mut config = Config::load_from_base_config_with_overrides( - ConfigToml::default(), - ConfigOverrides::default(), - codex_home.abs(), - ) - .await?; - let _ = config.features.enable(Feature::MemoryTool); - - let user_instructions = config - .user_instructions - .as_ref() - .expect("global instructions expected"); - assert_eq!(user_instructions.text(), "global instructions"); - assert_eq!( - user_instructions.sources().collect::>(), - vec![&global_agents_path] - ); - Ok(()) -} - -#[tokio::test] -async fn load_config_prefers_global_agents_override_instructions() -> std::io::Result<()> { - let codex_home = tempdir()?; - std::fs::write( - codex_home.path().join(DEFAULT_AGENTS_MD_FILENAME), - "global instructions", - )?; - let global_agents_override_path = codex_home.abs().join(LOCAL_AGENTS_MD_FILENAME); - std::fs::write(&global_agents_override_path, "local override instructions")?; - - let config = Config::load_from_base_config_with_overrides( - ConfigToml::default(), - ConfigOverrides::default(), - codex_home.abs(), - ) - .await?; - - let user_instructions = config - .user_instructions - .as_ref() - .expect("global override instructions expected"); - assert_eq!(user_instructions.text(), "local override instructions"); - assert_eq!( - user_instructions.sources().collect::>(), - vec![&global_agents_override_path] - ); - Ok(()) -} - #[tokio::test] async fn test_toml_parsing() { let history_with_persistence = r#" diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index fd74fcf2d251..f7b4c6293241 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1,5 +1,3 @@ -use crate::agents_md::AgentsMdManager; -pub use crate::agents_md::LoadedAgentsMd; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::path_utils::normalize_for_native_workdir; @@ -646,9 +644,6 @@ pub struct Config { /// Defaults to `false`. pub show_raw_agent_reasoning: bool, - /// User-provided instructions from AGENTS.md. - pub user_instructions: Option, - /// Base instructions override. pub base_instructions: Option, @@ -2602,12 +2597,6 @@ impl Config { .startup_warnings() .unwrap_or_default() .to_vec(); - let user_instructions = AgentsMdManager::load_global_instructions( - LOCAL_FS.as_ref(), - Some(&codex_home), - &mut startup_warnings, - ) - .await; // Destructure ConfigOverrides fully to ensure all overrides are applied. let ConfigOverrides { @@ -3454,7 +3443,6 @@ impl Config { approvals_reviewer: constrained_approvals_reviewer.value(), enforce_residency: enforce_residency.value, notify: cfg.notify, - user_instructions, base_instructions, personality, developer_instructions, diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 8504690b299a..c5b70c2a9510 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -29,7 +29,6 @@ use tokio::sync::Semaphore; use tokio_util::sync::CancellationToken; use tracing::warn; -use crate::LoadedAgentsMd; use crate::codex_delegate::run_codex_thread_interactive; use crate::config::Config; use crate::config::Constrained; @@ -147,7 +146,6 @@ struct GuardianReviewSessionReuseKey { permissions: Permissions, developer_instructions: Option, base_instructions: Option, - user_instructions: Option, compact_prompt: Option, cwd: AbsolutePathBuf, mcp_servers: Constrained>, @@ -172,7 +170,6 @@ impl GuardianReviewSessionReuseKey { permissions: spawn_config.permissions.clone(), developer_instructions: spawn_config.developer_instructions.clone(), base_instructions: spawn_config.base_instructions.clone(), - user_instructions: spawn_config.user_instructions.clone(), compact_prompt: spawn_config.compact_prompt.clone(), cwd: spawn_config.cwd.clone(), mcp_servers: spawn_config.mcp_servers.clone(), diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index bebe4f957eba..4161c06b93c8 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -179,7 +179,6 @@ async fn guardian_test_session_and_turn_with_base_url( session.thread_id = fixed_guardian_parent_session_id(); let mut config = (*turn.config).clone(); config.model_provider.base_url = Some(format!("{base_url}/v1")); - config.user_instructions = None; let config = Arc::new(config); let models_manager = test_support::models_manager_with_provider( config.codex_home.to_path_buf(), @@ -1993,7 +1992,6 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() -> crate::session::tests::make_session_and_context_with_rx().await; let mut config = (*turn.config).clone(); config.model_provider.base_url = Some(format!("{}/v1", server.uri())); - config.user_instructions = None; let config = Arc::new(config); let models_manager = test_support::models_manager_with_provider( config.codex_home.to_path_buf(), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 400c80368084..fbe7b537bda6 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -124,10 +124,8 @@ pub type NewConversation = NewThread; #[deprecated(note = "use CodexThread")] pub type CodexConversation = CodexThread; pub(crate) mod agents_md; -pub use agents_md::AgentsMdManager; pub use agents_md::DEFAULT_AGENTS_MD_FILENAME; pub use agents_md::LOCAL_AGENTS_MD_FILENAME; -pub use agents_md::LoadedAgentsMd; mod rollout; pub(crate) mod safety; mod session_rollout_init_error; diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index f6bf7d0022a4..70d7077644dc 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -19,7 +19,8 @@ use crate::session::turn::built_tools; use crate::state_db_bridge::StateDbHandle; use crate::thread_manager::ThreadManager; use crate::thread_manager::thread_store_from_config; -use codex_extension_api::empty_extension_registry; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_home::CodexHomeInstructionsContributor; /// Build the model-visible `input` list for a single debug turn. #[doc(hidden)] @@ -40,6 +41,10 @@ pub async fn build_prompt_input( let thread_store = thread_store_from_config(&config, state_db.clone()); let installation_id = resolve_installation_id(&config.codex_home).await?; + let mut extension_builder = ExtensionRegistryBuilder::::new(); + extension_builder.global_instructions_contributor(Arc::new( + CodexHomeInstructionsContributor::new(config.codex_home.clone()), + )); let thread_manager = ThreadManager::new( &config, Arc::clone(&auth_manager), @@ -52,7 +57,7 @@ pub async fn build_prompt_input( .await .map_err(|err| CodexErr::Fatal(err.to_string()))?, ), - empty_extension_registry(), + Arc::new(extension_builder.build()), /*analytics_events_client*/ None, thread_store, state_db.clone(), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 4e1609e70068..60c67971fef5 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -511,20 +511,27 @@ impl Codex { InitialHistory::New | InitialHistory::Cleared ); let user_instructions = if fresh_history { - if let Some(primary_environment) = environment_selections.primary_environment() { - let mut user_instruction_warnings = Vec::new(); - let loaded = AgentsMdManager::new(&config) - .user_instructions( - Some(primary_environment.as_ref()), - config.user_instructions.clone().unwrap_or_default(), - &mut user_instruction_warnings, - ) - .await; - config.startup_warnings.extend(user_instruction_warnings); - Some(loaded.unwrap_or_default()) - } else { - None - } + let global_instructions = + if let Some(contributor) = extensions.global_instructions_contributor() { + contributor.contribute().await.map_err(|err| { + CodexErr::Fatal(format!("failed to resolve global instructions: {err}")) + })? + } else { + codex_extension_api::GlobalInstructions::default() + }; + config + .startup_warnings + .extend(global_instructions.warnings.clone()); + let mut user_instruction_warnings = Vec::new(); + let loaded = AgentsMdManager::new(&config) + .user_instructions( + environment_selections.primary_environment().as_deref(), + LoadedAgentsMd::from_global(global_instructions), + &mut user_instruction_warnings, + ) + .await; + config.startup_warnings.extend(user_instruction_warnings); + Some(loaded.unwrap_or_default()) } else { None }; @@ -3090,47 +3097,70 @@ impl Session { } pub(crate) async fn refresh_global_instructions(&self, turn_context: &TurnContext) { - let configured_global = turn_context - .config - .user_instructions - .clone() - .unwrap_or_default(); - let needs_full_resolution = { - let state = self.state.lock().await; - state.session_configuration.user_instructions.is_none() + let contributor = self + .services + .extensions + .global_instructions_contributor() + .cloned(); + let resolved = match contributor { + Some(contributor) => contributor.contribute().await, + None => Ok(codex_extension_api::GlobalInstructions::default()), }; - if needs_full_resolution { - let Some(primary_environment) = turn_context.environments.primary_environment() else { + match resolved { + Ok(global_instructions) => { + for warning in &global_instructions.warnings { + self.send_event( + turn_context, + EventMsg::Warning(WarningEvent { + message: warning.clone(), + }), + ) + .await; + } + let needs_full_resolution = { + let state = self.state.lock().await; + state.session_configuration.user_instructions.is_none() + }; + if needs_full_resolution { + let mut warnings = Vec::new(); + let loaded = AgentsMdManager::new(turn_context.config.as_ref()) + .user_instructions( + turn_context.environments.primary_environment().as_deref(), + LoadedAgentsMd::from_global(global_instructions), + &mut warnings, + ) + .await + .unwrap_or_default(); + for warning in warnings { + self.send_event( + turn_context, + EventMsg::Warning(WarningEvent { message: warning }), + ) + .await; + } + let mut state = self.state.lock().await; + state.session_configuration.user_instructions = Some(loaded); + return; + } let mut state = self.state.lock().await; - state.session_configuration.user_instructions = Some(LoadedAgentsMd::default()); - return; - }; - let mut warnings = Vec::new(); - let loaded = AgentsMdManager::new(turn_context.config.as_ref()) - .user_instructions( - Some(primary_environment.as_ref()), - configured_global, - &mut warnings, - ) - .await - .unwrap_or_default(); - for warning in warnings { + let instructions = state + .session_configuration + .user_instructions + .get_or_insert_with(LoadedAgentsMd::default); + instructions.replace_global(global_instructions); + } + Err(err) => { self.send_event( turn_context, - EventMsg::Warning(WarningEvent { message: warning }), + EventMsg::Warning(WarningEvent { + message: format!( + "Failed to refresh global instructions; continuing with the previous snapshot: {err}" + ), + }), ) .await; } - let mut state = self.state.lock().await; - state.session_configuration.user_instructions = Some(loaded); - return; } - let mut state = self.state.lock().await; - let instructions = state - .session_configuration - .user_instructions - .get_or_insert_with(LoadedAgentsMd::default); - instructions.replace_global(configured_global); } pub(crate) async fn update_token_usage_info( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index e2e71c49e18f..02aa77ad4769 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -23,6 +23,11 @@ use codex_config::Sourced; use codex_config::loader::project_trust_key; use codex_config::types::ToolSuggestDisabledTool; +use codex_extension_api::ExtensionRegistry; +use codex_extension_api::GlobalInstruction; +use codex_extension_api::GlobalInstructions; +use codex_extension_api::GlobalInstructionsContributor; +use codex_extension_api::GlobalInstructionsFuture; use codex_features::Feature; use codex_login::CodexAuth; use codex_model_provider_info::ModelProviderInfo; @@ -170,6 +175,8 @@ use serde_json::json; use std::path::PathBuf; use std::sync::Arc; use std::sync::OnceLock; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; use std::time::Duration as StdDuration; mod guardian_tests; @@ -5042,6 +5049,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( initial_history: InitialHistory, session_source: SessionSource, agent_control: AgentControl, + extensions: Arc>, ) -> anyhow::Result<(Arc, async_channel::Receiver)> { let skip_global_instructions_refresh_on_first_turn = matches!( initial_history, @@ -5134,7 +5142,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( skills_manager, plugins_manager, mcp_manager, - Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), + extensions, agent_control, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, @@ -5158,6 +5166,23 @@ async fn make_session_with_history_source_and_agent_control_and_rx( Ok((session, rx_event)) } +struct CountingGlobalInstructionsContributor { + calls: AtomicUsize, +} + +impl GlobalInstructionsContributor for CountingGlobalInstructionsContributor { + fn contribute(&self) -> GlobalInstructionsFuture<'_> { + self.calls.fetch_add(1, Ordering::SeqCst); + Box::pin(std::future::ready(Ok(GlobalInstructions { + instructions: vec![GlobalInstruction::new( + "refreshed instructions", + /*source*/ None, + )], + warnings: Vec::new(), + }))) + } +} + #[tokio::test] async fn history_sharing_first_full_injection_uses_inherited_instruction_snapshot() { let inherited_snapshot = UserInstructionsSnapshot { @@ -5167,6 +5192,12 @@ async fn history_sharing_first_full_injection_uses_inherited_instruction_snapsho source: None, }], }; + let contributor = Arc::new(CountingGlobalInstructionsContributor { + calls: AtomicUsize::new(0), + }); + let mut extension_builder = codex_extension_api::ExtensionRegistryBuilder::::new(); + extension_builder.global_instructions_contributor(contributor.clone()); + let extensions = Arc::new(extension_builder.build()); let (session, _rx_event) = make_session_with_history_source_and_agent_control_and_rx( InitialHistory::Forked(vec![RolloutItem::TurnContext(TurnContextItem { turn_id: None, @@ -5191,10 +5222,12 @@ async fn history_sharing_first_full_injection_uses_inherited_instruction_snapsho })]), SessionSource::Exec, AgentControl::default(), + extensions, ) .await .expect("forked session should start"); + assert_eq!(contributor.calls.load(Ordering::SeqCst), 0); assert!(session.reference_context_item().await.is_none()); let first_turn = session.new_default_turn().await; @@ -5211,6 +5244,88 @@ async fn history_sharing_first_full_injection_uses_inherited_instruction_snapsho .map(LoadedAgentsMd::snapshot) }; assert_eq!(first_turn_snapshot, Some(inherited_snapshot)); + assert_eq!(contributor.calls.load(Ordering::SeqCst), 0); + + { + let mut state = session.state.lock().await; + state.set_reference_context_item(/*item*/ None); + } + let later_turn = session.new_default_turn().await; + session + .record_context_updates_and_set_reference_context_item(later_turn.as_ref()) + .await; + + let refreshed_snapshot = { + let state = session.state.lock().await; + state + .session_configuration + .user_instructions + .as_ref() + .map(LoadedAgentsMd::snapshot) + }; + assert_eq!( + refreshed_snapshot, + Some(UserInstructionsSnapshot { + instructions: vec![InstructionSnapshot { + contents: "refreshed instructions".to_string(), + provenance: InstructionProvenance::Global, + source: None, + }], + }) + ); + assert_eq!(contributor.calls.load(Ordering::SeqCst), 1); +} + +#[tokio::test] +async fn legacy_history_refreshes_when_first_turn_requires_full_context() { + let contributor = Arc::new(CountingGlobalInstructionsContributor { + calls: AtomicUsize::new(0), + }); + let mut extension_builder = codex_extension_api::ExtensionRegistryBuilder::::new(); + extension_builder.global_instructions_contributor(contributor.clone()); + let (session, _rx_event) = make_session_with_history_source_and_agent_control_and_rx( + InitialHistory::Forked(Vec::new()), + SessionSource::Exec, + AgentControl::default(), + Arc::new(extension_builder.build()), + ) + .await + .expect("legacy forked session should start"); + + assert_eq!(contributor.calls.load(Ordering::SeqCst), 0); + let turn_context = session.new_default_turn().await; + let mut warnings = Vec::new(); + let mut expected = AgentsMdManager::new(turn_context.config.as_ref()) + .user_instructions( + turn_context.environments.primary_environment().as_deref(), + LoadedAgentsMd::default(), + &mut warnings, + ) + .await + .expect("project instructions should load"); + assert_eq!(warnings, Vec::::new()); + expected.replace_global(GlobalInstructions { + instructions: vec![GlobalInstruction::new( + "refreshed instructions", + /*source*/ None, + )], + warnings: Vec::new(), + }); + + session + .record_context_updates_and_set_reference_context_item(turn_context.as_ref()) + .await; + + let snapshot = { + let state = session.state.lock().await; + state + .session_configuration + .user_instructions + .as_ref() + .map(LoadedAgentsMd::snapshot) + }; + assert_eq!(snapshot, Some(expected.snapshot())); + assert_eq!(contributor.calls.load(Ordering::SeqCst), 1); } #[tokio::test] @@ -5224,6 +5339,7 @@ async fn resumed_root_session_uses_thread_id_as_session_id() { }), SessionSource::Exec, AgentControl::default(), + Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), ) .await .expect("resume should succeed"); @@ -5259,6 +5375,7 @@ async fn resumed_subagent_session_keeps_inherited_session_id() { }), session_source, AgentControl::default().with_session_id(parent_session_id, /*max_threads*/ usize::MAX), + Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), ) .await .expect("resume should succeed"); diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 0d5903693634..9d147d448bad 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -1,5 +1,4 @@ use super::*; -use crate::LoadedAgentsMd; use crate::ThreadManager; use crate::config::AgentRoleConfig; use crate::config::DEFAULT_AGENT_MAX_DEPTH; @@ -4516,24 +4515,6 @@ async fn build_agent_spawn_config_uses_turn_context_values() { assert_eq!(config, expected); } -#[tokio::test] -async fn build_agent_spawn_config_preserves_base_user_instructions() { - let (_session, mut turn) = make_session_and_context().await; - let mut base_config = (*turn.config).clone(); - base_config.user_instructions = Some(LoadedAgentsMd::new_user( - "base-user".to_string(), - base_config.codex_home.join("AGENTS.md"), - )); - turn.config = Arc::new(base_config.clone()); - let base_instructions = BaseInstructions { - text: "base".to_string(), - }; - - let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config"); - - assert_eq!(config.user_instructions, base_config.user_instructions); -} - #[tokio::test] async fn build_agent_resume_config_clears_base_instructions() { let (_session, mut turn) = make_session_and_context().await; diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 1e4313590023..c6093250550f 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use std::process::Command; use std::sync::Arc; use std::sync::atomic::AtomicU64; +use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use std::time::Duration; @@ -24,6 +25,11 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::GlobalInstruction; +use codex_extension_api::GlobalInstructions; +use codex_extension_api::GlobalInstructionsContributor; +use codex_extension_api::GlobalInstructionsFuture; use codex_extension_api::empty_extension_registry; use codex_login::CodexAuth; use codex_model_provider_info::ModelProviderInfo; @@ -221,6 +227,48 @@ pub struct TestCodexBuilder { extensions: Arc>, } +pub struct TestGlobalInstructionsContributor { + responses: Vec>, + calls: AtomicUsize, +} + +impl TestGlobalInstructionsContributor { + pub fn new(responses: Vec>) -> Self { + assert!( + !responses.is_empty(), + "test contributor requires at least one response" + ); + Self { + responses, + calls: AtomicUsize::new(0), + } + } + + pub fn static_instructions(contents: impl Into) -> Self { + Self::new(vec![Ok(GlobalInstructions { + instructions: vec![GlobalInstruction::new(contents, /*source*/ None)], + warnings: Vec::new(), + })]) + } + + pub fn calls(&self) -> usize { + self.calls.load(Ordering::SeqCst) + } +} + +impl GlobalInstructionsContributor for TestGlobalInstructionsContributor { + fn contribute(&self) -> GlobalInstructionsFuture<'_> { + let call = self.calls.fetch_add(1, Ordering::SeqCst); + let response = self + .responses + .get(call) + .or_else(|| self.responses.last()) + .expect("test contributor responses are non-empty") + .clone(); + Box::pin(std::future::ready(response)) + } +} + impl TestCodexBuilder { pub fn with_config(mut self, mutator: T) -> Self where @@ -308,6 +356,14 @@ impl TestCodexBuilder { self } + pub fn with_global_instructions(self, contents: impl Into) -> Self { + let mut builder = ExtensionRegistryBuilder::::new(); + builder.global_instructions_contributor(Arc::new( + TestGlobalInstructionsContributor::static_instructions(contents), + )); + self.with_extensions(Arc::new(builder.build())) + } + pub fn with_windows_cmd_shell(self) -> Self { if cfg!(windows) { self.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("cmd.exe"))) diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index 3fa2136ecbd9..deac61f81207 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -1,5 +1,8 @@ use anyhow::Result; +use codex_core::config::Config; use codex_exec_server::CreateDirectoryOptions; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_home::CodexHomeInstructionsContributor; use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::create_directory_symlink; use core_test_support::responses::ev_completed; @@ -236,9 +239,16 @@ async fn selected_environment_sources_match_model_visible_instructions() -> Resu let home = Arc::new(TempDir::new()?); let global_agents = home.path().join("AGENTS.md"); std::fs::write(&global_agents, "global doc")?; + let mut extension_builder = ExtensionRegistryBuilder::::new(); + extension_builder.global_instructions_contributor(Arc::new( + CodexHomeInstructionsContributor::new(AbsolutePathBuf::try_from( + home.path().to_path_buf(), + )?), + )); let mut builder = test_codex() .with_home(home) + .with_extensions(Arc::new(extension_builder.build())) .with_workspace_setup(|cwd, fs| async move { fs.write_file( &cwd.join("AGENTS.md"), diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 2fcb73e25a20..17154ad6ad02 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1,6 +1,5 @@ use codex_config::ConfigLayerStack; use codex_config::types::AuthCredentialsStoreMode; -use codex_core::LoadedAgentsMd; use codex_core::ModelClient; use codex_core::NewThread; use codex_core::Prompt; @@ -363,10 +362,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { let codex_home = Arc::new(TempDir::new().unwrap()); let mut builder = test_codex() .with_home(codex_home.clone()) - .with_config(|config| { - // Ensure user instructions are NOT delivered on resume. - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing("be nice")); - }); + .with_global_instructions("be nice"); let test = builder .resume(&server, codex_home, session_path.clone()) .await @@ -1180,9 +1176,7 @@ async fn includes_user_instructions_message_in_request() { let mut builder = test_codex() .with_auth(CodexAuth::from_api_key("Test API Key")) - .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing("be nice")); - }); + .with_global_instructions("be nice"); let codex = builder .build(&server) .await @@ -2267,8 +2261,8 @@ async fn includes_developer_instructions_message_in_request() { .await; let mut builder = test_codex() .with_auth(CodexAuth::from_api_key("Test API Key")) + .with_global_instructions("be nice") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing("be nice")); config.developer_instructions = Some("be useful".to_string()); }); let codex = builder diff --git a/codex-rs/core/tests/suite/compact_remote_parity.rs b/codex-rs/core/tests/suite/compact_remote_parity.rs index 64bac1468cf6..6763c3c8bc79 100644 --- a/codex-rs/core/tests/suite/compact_remote_parity.rs +++ b/codex-rs/core/tests/suite/compact_remote_parity.rs @@ -4,9 +4,14 @@ use std::fs; use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; use anyhow::Result; -use codex_core::LoadedAgentsMd; +use codex_core::config::Config; +use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::GlobalInstruction; +use codex_extension_api::GlobalInstructions; use codex_features::Feature; use codex_login::CodexAuth; use codex_protocol::config_types::ServiceTier; @@ -21,6 +26,7 @@ use core_test_support::responses::ResponseMock; use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::TestCodexHarness; +use core_test_support::test_codex::TestGlobalInstructionsContributor; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; @@ -34,7 +40,8 @@ const FIXED_CWD: &str = "/tmp/codex_remote_compaction_parity_workspace"; const IMAGE_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII="; const SUMMARY: &str = "REMOTE_COMPACTION_PARITY_ENCRYPTED_SUMMARY"; const DUMMY_FUNCTION_NAME: &str = "test_tool"; -const USER_INSTRUCTIONS: &str = "PARITY_USER_INSTRUCTIONS"; +const INITIAL_USER_INSTRUCTIONS: &str = "PARITY_USER_INSTRUCTIONS"; +const REFRESHED_USER_INSTRUCTIONS: &str = "PARITY_REFRESHED_USER_INSTRUCTIONS"; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Mode { @@ -327,11 +334,11 @@ fn assert_capture_eq( &legacy_follow_up, &v2_follow_up, ); - assert_single_user_instruction_message( + assert_single_refreshed_user_instruction_message( &legacy.follow_up_body, &format!("legacy follow-up for {label}"), ); - assert_single_user_instruction_message( + assert_single_refreshed_user_instruction_message( &v2.follow_up_body, &format!("v2 follow-up for {label}"), ); @@ -341,15 +348,17 @@ fn assert_capture_eq( &legacy.replacement_history, &v2.replacement_history, ); - assert_user_instruction_messages_in_items( + assert_user_instruction_messages_in_items_with_contents( &legacy.replacement_history, &format!("legacy replacement history for {label}"), replacement_instruction_messages, + REFRESHED_USER_INSTRUCTIONS, ); - assert_user_instruction_messages_in_items( + assert_user_instruction_messages_in_items_with_contents( &v2.replacement_history, &format!("v2 replacement history for {label}"), replacement_instruction_messages, + REFRESHED_USER_INSTRUCTIONS, ); println!( @@ -403,11 +412,11 @@ fn assert_follow_up_and_history_eq( &legacy_follow_up, &v2_follow_up, ); - assert_single_user_instruction_message( + assert_single_refreshed_user_instruction_message( &legacy.follow_up_body, &format!("legacy follow-up for {label}"), ); - assert_single_user_instruction_message( + assert_single_refreshed_user_instruction_message( &v2.follow_up_body, &format!("v2 follow-up for {label}"), ); @@ -638,7 +647,8 @@ async fn run_persisted_history_resume(mode: Mode) -> Result TestCodexBu FIXED_CWD, )) .expect("fixed cwd should be absolute"); - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing(USER_INSTRUCTIONS)); config.developer_instructions = Some("PARITY_DEVELOPER_INSTRUCTIONS".to_string()); if settings.service_tier_fast { config.service_tier = Some(ServiceTier::Fast.request_value().to_string()); @@ -775,6 +785,28 @@ fn prepare_fixed_cwd() -> Result<()> { Ok(()) } +fn global_instruction_extensions() -> Arc> { + let contributor = TestGlobalInstructionsContributor::new(vec![ + Ok(GlobalInstructions { + instructions: vec![GlobalInstruction::new( + INITIAL_USER_INSTRUCTIONS, + /*source*/ None, + )], + warnings: Vec::new(), + }), + Ok(GlobalInstructions { + instructions: vec![GlobalInstruction::new( + REFRESHED_USER_INSTRUCTIONS, + /*source*/ None, + )], + warnings: Vec::new(), + }), + ]); + let mut builder = ExtensionRegistryBuilder::new(); + builder.global_instructions_contributor(Arc::new(contributor)); + Arc::new(builder.build()) +} + fn rollout_path(harness: &TestCodexHarness) -> PathBuf { harness .test() @@ -1017,21 +1049,48 @@ fn follow_up_request_view(body: &Value) -> Value { } fn assert_single_user_instruction_message(body: &Value, label: &str) { + assert_single_user_instruction_message_with_contents(body, label, INITIAL_USER_INSTRUCTIONS); +} + +fn assert_single_refreshed_user_instruction_message(body: &Value, label: &str) { + assert_single_user_instruction_message_with_contents(body, label, REFRESHED_USER_INSTRUCTIONS); +} + +fn assert_single_user_instruction_message_with_contents( + body: &Value, + label: &str, + expected_instructions: &str, +) { let input = body .get("input") .and_then(Value::as_array) .cloned() .unwrap_or_default(); - assert_user_instruction_messages_in_items( + assert_user_instruction_messages_in_items_with_contents( &Value::Array(input), label, /*expected_count*/ 1, + expected_instructions, ); } fn assert_user_instruction_messages_in_items(items: &Value, label: &str, expected_count: usize) { + assert_user_instruction_messages_in_items_with_contents( + items, + label, + expected_count, + INITIAL_USER_INSTRUCTIONS, + ); +} + +fn assert_user_instruction_messages_in_items_with_contents( + items: &Value, + label: &str, + expected_count: usize, + expected_instructions: &str, +) { let expected_text = format!( - "# AGENTS.md instructions for {FIXED_CWD}\n\n\n{USER_INSTRUCTIONS}\n" + "# AGENTS.md instructions for {FIXED_CWD}\n\n\n{expected_instructions}\n" ); let instruction_messages = items .as_array() diff --git a/codex-rs/core/tests/suite/global_instructions.rs b/codex-rs/core/tests/suite/global_instructions.rs index 8041d6742f90..89ceba94fda0 100644 --- a/codex-rs/core/tests/suite/global_instructions.rs +++ b/codex-rs/core/tests/suite/global_instructions.rs @@ -6,7 +6,13 @@ use std::time::Duration; use anyhow::Result; use anyhow::anyhow; use codex_core::ForkSnapshot; +use codex_core::config::Config; +use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::GlobalInstruction; +use codex_extension_api::GlobalInstructions; use codex_features::Feature; +use codex_home::CodexHomeInstructionsContributor; use codex_login::CodexAuth; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::built_in_model_providers; @@ -19,6 +25,8 @@ use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::load_default_config_for_test; use core_test_support::responses; use core_test_support::skip_if_no_network; +use core_test_support::test_codex::TestCodexBuilder; +use core_test_support::test_codex::TestGlobalInstructionsContributor; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; @@ -60,6 +68,39 @@ fn write_global_file( AbsolutePathBuf::try_from(path).map_err(Into::into) } +fn test_codex_with_home(home: Arc) -> Result { + let codex_home = AbsolutePathBuf::try_from(home.path().to_path_buf())?; + let mut extensions = ExtensionRegistryBuilder::::new(); + extensions.global_instructions_contributor(Arc::new(CodexHomeInstructionsContributor::new( + codex_home, + ))); + Ok(test_codex() + .with_home(home) + .with_extensions(Arc::new(extensions.build()))) +} + +fn test_instruction_extensions( + contributor: Arc, +) -> Arc> { + let mut extensions = ExtensionRegistryBuilder::::new(); + extensions.global_instructions_contributor(contributor); + Arc::new(extensions.build()) +} + +fn contributed_instructions( + contents: &str, + source: Option, + warnings: &[&str], +) -> GlobalInstructions { + GlobalInstructions { + instructions: vec![GlobalInstruction::new(contents, source)], + warnings: warnings + .iter() + .map(|warning| (*warning).to_string()) + .collect(), + } +} + fn instruction_fragments(request: &responses::ResponsesRequest) -> Vec { request .message_input_texts("user") @@ -204,9 +245,8 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re let home = Arc::new(TempDir::new()?); let global_source = write_global(home.as_ref(), GLOBAL_INSTRUCTIONS)?; - let mut builder = test_codex() - .with_home(Arc::clone(&home)) - .with_workspace_setup(|cwd, fs| async move { + let mut builder = + test_codex_with_home(Arc::clone(&home))?.with_workspace_setup(|cwd, fs| async move { fs.write_file( &cwd.join("AGENTS.md"), PROJECT_INSTRUCTIONS.as_bytes().to_vec(), @@ -305,8 +345,7 @@ async fn global_instruction_context_item_is_currently_not_limited_by_project_doc let home = Arc::new(TempDir::new()?); let oversized_global = vec!["global instruction item remains uncapped"; 512].join("\n"); let source = write_global(home.as_ref(), &oversized_global)?; - let mut builder = test_codex() - .with_home(Arc::clone(&home)) + let mut builder = test_codex_with_home(Arc::clone(&home))? .with_config(|config| config.project_doc_max_bytes = 1); let test = builder.build(&server).await?; @@ -346,7 +385,7 @@ async fn global_loading_warning_surfaces_during_thread_creation() -> Result<()> let source = write_global(home.as_ref(), b"global\xFFinstructions")?; // Create the thread, capture its load warning, and submit one turn for rendered output. - let mut builder = test_codex().with_home(home); + let mut builder = test_codex_with_home(home)?; let test = builder.build(&server).await?; let warning = wait_for_event_match(&test.codex, |event| match event { EventMsg::Warning(warning) @@ -375,6 +414,50 @@ async fn global_loading_warning_surfaces_during_thread_creation() -> Result<()> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn contributor_warnings_surface_and_failure_aborts_fresh_thread_creation() -> Result<()> { + let server = responses::start_mock_server().await; + let warning_contributor = Arc::new(TestGlobalInstructionsContributor::new(vec![Ok( + contributed_instructions( + "global instructions", + /*source*/ None, + &["global warning"], + ), + )])); + let mut warning_builder = test_codex().with_extensions(test_instruction_extensions( + Arc::clone(&warning_contributor), + )); + let warning_test = warning_builder.build(&server).await?; + + let warning = wait_for_event_match(&warning_test.codex, |event| match event { + EventMsg::Warning(warning) if warning.message == "global warning" => { + Some(warning.message.clone()) + } + _ => None, + }) + .await; + assert_eq!(warning, "global warning"); + assert_eq!(warning_contributor.calls(), 1); + + let failure_contributor = Arc::new(TestGlobalInstructionsContributor::new(vec![Err( + "contributor failed".to_string(), + )])); + let mut failure_builder = test_codex().with_extensions(test_instruction_extensions( + Arc::clone(&failure_contributor), + )); + let error = match failure_builder.build(&server).await { + Ok(_) => panic!("thread creation should fail"), + Err(error) => error, + }; + assert!( + error.to_string().contains("contributor failed"), + "unexpected creation error: {error:#}" + ); + assert_eq!(failure_contributor.calls(), 1); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn cold_resume_restores_persisted_instruction_sources() -> Result<()> { // Set up an initial turn and a later cold-resumed turn against the same rollout. @@ -397,7 +480,7 @@ async fn cold_resume_restores_persisted_instruction_sources() -> Result<()> { let old_source = write_global(home.as_ref(), OLD_GLOBAL_INSTRUCTIONS)?; // Create the initial thread and persist its creation-time instruction snapshot. - let mut initial_builder = test_codex().with_home(Arc::clone(&home)); + let mut initial_builder = test_codex_with_home(Arc::clone(&home))?; let initial = initial_builder.build(&server).await?; // Assert the pre-resume thread reports the source used to create its snapshot. @@ -421,7 +504,7 @@ async fn cold_resume_restores_persisted_instruction_sources() -> Result<()> { // Add a preferred override source, then cold-resume with freshly loaded configuration. let new_source = write_global_override(home.as_ref(), NEW_GLOBAL_INSTRUCTIONS)?; assert_ne!(old_source, new_source); - let mut resume_builder = test_codex().with_home(Arc::clone(&home)); + let mut resume_builder = test_codex_with_home(Arc::clone(&home))?; let resumed = resume_builder .resume(&server, Arc::clone(&home), rollout_path) .await?; @@ -474,7 +557,7 @@ async fn fork_replays_rendered_instructions_from_shared_history() -> Result<()> let source = write_global(home.as_ref(), OLD_GLOBAL_INSTRUCTIONS)?; // Create the parent and persist its creation-time instruction snapshot. - let mut builder = test_codex().with_home(Arc::clone(&home)); + let mut builder = test_codex_with_home(Arc::clone(&home))?; let parent = builder.build(&server).await?; // Assert the parent reports the source used to create its snapshot. @@ -628,12 +711,10 @@ async fn run_subagent_global_instruction_case(fork_context: bool) -> Result<()> // Create the parent thread, record its source, and seed the history inherited by the child. let home = Arc::new(TempDir::new()?); let source = write_global(home.as_ref(), OLD_GLOBAL_INSTRUCTIONS)?; - let mut builder = test_codex() - .with_home(Arc::clone(&home)) - .with_config(|config| { - let _ = config.features.enable(Feature::Collab); - let _ = config.features.disable(Feature::EnableRequestCompression); - }); + let mut builder = test_codex_with_home(Arc::clone(&home))?.with_config(|config| { + let _ = config.features.enable(Feature::Collab); + let _ = config.features.disable(Feature::EnableRequestCompression); + }); let test = builder.build(&server).await?; // Assert the parent reports the creation-time source before spawning. @@ -671,12 +752,20 @@ async fn run_subagent_global_instruction_case(fork_context: bool) -> Result<()> .await .map_err(|_| anyhow!("timed out waiting for the subagent request"))?; - // Assert parent and child report and render the parent's creation-time snapshot exactly once. - let expected_fragment = - expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); - assert_single_instruction_fragment(&seed_request, &expected_fragment); - assert_single_instruction_fragment(&spawn_request, &expected_fragment); - assert_single_instruction_fragment(&child_request, &expected_fragment); + // The parent and forked child inherit the creation-time snapshot. A fresh child resolves the + // contributor again and sees the new preferred override. + let parent_fragment = expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); + let (child_source, child_fragment) = if fork_context { + (source.clone(), parent_fragment.clone()) + } else { + ( + new_source, + expected_instruction_fragment(&test.config.cwd, NEW_GLOBAL_INSTRUCTIONS), + ) + }; + assert_single_instruction_fragment(&seed_request, &parent_fragment); + assert_single_instruction_fragment(&spawn_request, &parent_fragment); + assert_single_instruction_fragment(&child_request, &child_fragment); assert_eq!( test.codex.instruction_sources().await, vec![source.clone()], @@ -684,8 +773,8 @@ async fn run_subagent_global_instruction_case(fork_context: bool) -> Result<()> ); assert_eq!( child_thread.instruction_sources().await, - vec![source], - "subagent reports the parent's creation-time source" + vec![child_source], + "subagent reports the source selected for its history mode" ); if fork_context { let seed_input = seed_request.input(); @@ -719,7 +808,7 @@ async fn run_subagent_global_instruction_case(fork_context: bool) -> Result<()> } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn manual_compaction_keeps_the_creation_time_global_instructions() -> Result<()> { +async fn manual_compaction_refreshes_global_instructions_on_the_next_full_rebuild() -> Result<()> { // Set up an initial turn, a manual compaction response, and a post-compaction turn. let server = responses::start_mock_server().await; let response_mock = responses::mount_sse_sequence( @@ -746,11 +835,9 @@ async fn manual_compaction_keeps_the_creation_time_global_instructions() -> Resu let provider = local_compaction_provider(&server); // Create the thread with the old global source loaded into its instruction snapshot. - let mut builder = test_codex() - .with_home(Arc::clone(&home)) - .with_config(move |config| { - config.model_provider = provider; - }); + let mut builder = test_codex_with_home(Arc::clone(&home))?.with_config(move |config| { + config.model_provider = provider; + }); let test = builder.build(&server).await?; // Assert the pre-compaction source list points at the creation-time file. @@ -772,26 +859,26 @@ async fn manual_compaction_keeps_the_creation_time_global_instructions() -> Resu .await; test.submit_turn("after compact").await?; - // Assert ordinary and compact turns keep the old rendering even though the reported source - // path now contains new text. + // The compact request uses the existing snapshot. The following full-context rebuild resolves + // the contributor again and installs the current contents. let requests = response_mock.requests(); assert_eq!(requests.len(), 3); - let expected_fragment = - expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); - assert_single_instruction_fragment(&requests[0], &expected_fragment); - assert_single_instruction_fragment(&requests[1], &expected_fragment); - assert_single_instruction_fragment(&requests[2], &expected_fragment); + let old_fragment = expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); + let new_fragment = expected_instruction_fragment(&test.config.cwd, NEW_GLOBAL_INSTRUCTIONS); + assert_single_instruction_fragment(&requests[0], &old_fragment); + assert_single_instruction_fragment(&requests[1], &old_fragment); + assert_single_instruction_fragment(&requests[2], &new_fragment); assert_eq!( test.codex.instruction_sources().await, vec![source], - "thread retains the creation-time global source after compaction" + "same-path refresh keeps the selected source path" ); Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn mid_turn_compaction_keeps_the_creation_time_global_instructions() -> Result<()> { +async fn mid_turn_compaction_refreshes_global_instructions_for_the_resumed_request() -> Result<()> { // Set up a turn that crosses the auto-compaction limit and a post-compaction response. let server = responses::start_mock_server().await; let response_mock = responses::mount_sse_sequence( @@ -817,13 +904,11 @@ async fn mid_turn_compaction_keeps_the_creation_time_global_instructions() -> Re let provider = local_compaction_provider(&server); // Create the thread with the old global source loaded into its instruction snapshot. - let mut builder = test_codex() - .with_home(Arc::clone(&home)) - .with_config(move |config| { - config.model_provider = provider; - config.model_context_window = Some(100); - config.model_auto_compact_token_limit = Some(90); - }); + let mut builder = test_codex_with_home(Arc::clone(&home))?.with_config(move |config| { + config.model_provider = provider; + config.model_context_window = Some(100); + config.model_auto_compact_token_limit = Some(90); + }); let test = builder.build(&server).await?; // Assert the pre-compaction source list points at the creation-time file. @@ -838,18 +923,19 @@ async fn mid_turn_compaction_keeps_the_creation_time_global_instructions() -> Re assert_ne!(source, new_source); test.submit_turn("trigger mid-turn compaction").await?; - // Assert the initial, compact, and resumed requests all keep the old snapshot and source. + // The initial and compact requests use the old snapshot. The resumed request resolves the + // contributor again and adopts the preferred override. let requests = response_mock.requests(); assert_eq!(requests.len(), 3); - let expected_fragment = - expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); - assert_single_instruction_fragment(&requests[0], &expected_fragment); - assert_single_instruction_fragment(&requests[1], &expected_fragment); - assert_single_instruction_fragment(&requests[2], &expected_fragment); + let old_fragment = expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); + let new_fragment = expected_instruction_fragment(&test.config.cwd, NEW_GLOBAL_INSTRUCTIONS); + assert_single_instruction_fragment(&requests[0], &old_fragment); + assert_single_instruction_fragment(&requests[1], &old_fragment); + assert_single_instruction_fragment(&requests[2], &new_fragment); assert_eq!( test.codex.instruction_sources().await, - vec![source], - "thread retains the creation-time global source after mid-turn compaction" + vec![new_source], + "mid-turn full-context rebuild reports the refreshed source" ); Ok(()) @@ -859,6 +945,85 @@ async fn mid_turn_compaction_keeps_the_creation_time_global_instructions() -> Re // later full-context rebuilds. Reloading file contents into historical context currently rewrites // model-visible history and invalidates the cached prefix; decide whether the original item should // remain stable instead. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mid_turn_refresh_failure_warns_and_retains_the_previous_snapshot() -> Result<()> { + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_sequence( + &server, + vec![ + responses::sse(vec![ + responses::ev_function_call("call-1", "unsupported_tool", "{}"), + responses::ev_completed_with_tokens("first-response", /*total_tokens*/ 96), + ]), + responses::sse(vec![ + responses::ev_assistant_message("compact-message", "summary"), + responses::ev_completed_with_tokens("compact-response", /*total_tokens*/ 10), + ]), + responses::sse(vec![ + responses::ev_assistant_message("final-message", "done"), + responses::ev_completed_with_tokens("follow-up-response", /*total_tokens*/ 10), + ]), + ], + ) + .await; + let contributor = Arc::new(TestGlobalInstructionsContributor::new(vec![ + Ok(contributed_instructions( + OLD_GLOBAL_INSTRUCTIONS, + /*source*/ None, + &[], + )), + Err("refresh failed".to_string()), + ])); + let provider = local_compaction_provider(&server); + let mut builder = test_codex() + .with_extensions(test_instruction_extensions(Arc::clone(&contributor))) + .with_config(move |config| { + config.model_provider = provider; + config.model_context_window = Some(100); + config.model_auto_compact_token_limit = Some(90); + }); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "trigger mid-turn compaction".to_string(), + text_elements: Vec::new(), + }], + environments: None, + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + let warning = wait_for_event_match(&test.codex, |event| match event { + EventMsg::Warning(warning) if warning.message.contains("refresh failed") => { + Some(warning.message.clone()) + } + _ => None, + }) + .await; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert!( + warning.contains("continuing with the previous snapshot"), + "unexpected refresh warning: {warning}" + ); + assert_eq!(contributor.calls(), 2); + let requests = response_mock.requests(); + assert_eq!(requests.len(), 3); + let old_fragment = expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); + assert_single_instruction_fragment(&requests[0], &old_fragment); + assert_single_instruction_fragment(&requests[1], &old_fragment); + assert_single_instruction_fragment(&requests[2], &old_fragment); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn cold_resume_then_full_context_rebuild_uses_current_instructions() -> Result<()> { // Set up an initial turn, a cold-resumed turn, manual compaction, and the later full-context @@ -892,7 +1057,7 @@ async fn cold_resume_then_full_context_rebuild_uses_current_instructions() -> Re let source = write_global(home.as_ref(), OLD_GLOBAL_INSTRUCTIONS)?; // Create the initial thread and persist its creation-time instruction snapshot. - let mut initial_builder = test_codex().with_home(Arc::clone(&home)).with_config({ + let mut initial_builder = test_codex_with_home(Arc::clone(&home))?.with_config({ let provider = provider.clone(); move |config| config.model_provider = provider }); @@ -919,8 +1084,7 @@ async fn cold_resume_then_full_context_rebuild_uses_current_instructions() -> Re // Rewrite the selected AGENTS.md in place, then cold-resume with freshly loaded configuration. let rewritten_source = write_global(home.as_ref(), NEW_GLOBAL_INSTRUCTIONS)?; assert_eq!(source, rewritten_source); - let mut resume_builder = test_codex() - .with_home(Arc::clone(&home)) + let mut resume_builder = test_codex_with_home(Arc::clone(&home))? .with_config(move |config| config.model_provider = provider); let resumed = resume_builder .resume(&server, Arc::clone(&home), rollout_path) @@ -1003,7 +1167,7 @@ async fn legacy_compaction_without_replacement_history_rebuilds_current_instruct let provider = local_compaction_provider(&server); let home = Arc::new(TempDir::new()?); let source = write_global(home.as_ref(), OLD_GLOBAL_INSTRUCTIONS)?; - let mut initial_builder = test_codex().with_home(Arc::clone(&home)).with_config({ + let mut initial_builder = test_codex_with_home(Arc::clone(&home))?.with_config({ let provider = provider.clone(); move |config| config.model_provider = provider }); @@ -1032,8 +1196,7 @@ async fn legacy_compaction_without_replacement_history_rebuilds_current_instruct // Rewrite the selected file in place and cold-resume with current configuration. let rewritten_source = write_global(home.as_ref(), NEW_GLOBAL_INSTRUCTIONS)?; assert_eq!(source, rewritten_source); - let mut resume_builder = test_codex() - .with_home(Arc::clone(&home)) + let mut resume_builder = test_codex_with_home(Arc::clone(&home))? .with_config(move |config| config.model_provider = provider); let resumed = resume_builder .resume(&server, Arc::clone(&home), rollout_path) @@ -1086,8 +1249,7 @@ async fn remote_v2_compaction_keeps_creation_time_instructions_after_same_path_m .await; let home = Arc::new(TempDir::new()?); let source = write_global(home.as_ref(), OLD_GLOBAL_INSTRUCTIONS)?; - let mut builder = test_codex() - .with_home(Arc::clone(&home)) + let mut builder = test_codex_with_home(Arc::clone(&home))? .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) .with_config(|config| { let _ = config.features.enable(Feature::RemoteCompactionV2); @@ -1106,14 +1268,15 @@ async fn remote_v2_compaction_keeps_creation_time_instructions_after_same_path_m test.submit_turn("after remote v2 compaction").await?; test.codex.flush_rollout().await?; - // Assert the compact request, installed replacement history, and follow-up all keep the - // creation-time item despite the file-backed source now containing new text. + // The compact request retains the old snapshot. The post-compact full-context rebuild resolves + // the contributor again and installs the rewritten contents. let requests = response_mock.requests(); assert_eq!(requests.len(), 3); let old_fragment = expected_instruction_fragment(&test.config.cwd, OLD_GLOBAL_INSTRUCTIONS); + let new_fragment = expected_instruction_fragment(&test.config.cwd, NEW_GLOBAL_INSTRUCTIONS); assert_single_instruction_fragment(&requests[0], &old_fragment); assert_single_instruction_fragment(&requests[1], &old_fragment); - assert_single_instruction_fragment(&requests[2], &old_fragment); + assert_single_instruction_fragment(&requests[2], &new_fragment); assert_eq!( requests[1].input().last(), Some(&json!({"type": "compaction_trigger"})), @@ -1144,8 +1307,7 @@ async fn remote_v2_compaction_keeps_creation_time_instructions_after_same_path_m }) .await; let resumed_cwd = test.config.cwd.clone(); - let mut resume_builder = test_codex() - .with_home(Arc::clone(&home)) + let mut resume_builder = test_codex_with_home(Arc::clone(&home))? .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) .with_config(move |config| { config.cwd = resumed_cwd; @@ -1158,11 +1320,11 @@ async fn remote_v2_compaction_keeps_creation_time_instructions_after_same_path_m .submit_turn("after remote v2 compaction cold resume") .await?; - // Modern replacement-history resume replays the persisted checkpoint and its later old-context - // suffix even though the same source path now contains new text. + // Modern replacement-history resume replays the persisted checkpoint and its later refreshed + // full-context suffix. let requests = response_mock.requests(); assert_eq!(requests.len(), 4); - assert_single_instruction_fragment(&requests[3], &old_fragment); + assert_single_instruction_fragment(&requests[3], &new_fragment); let resumed_input = requests[3].input(); assert_eq!( resumed_input.get(..replacement_history.len()), diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 640c1c1db1aa..650a9cc21402 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -1,6 +1,5 @@ #![allow(clippy::unwrap_used)] -use codex_core::LoadedAgentsMd; use codex_core::shell::default_user_shell; use codex_features::Feature; use codex_prompts::APPLY_PATCH_TOOL_INSTRUCTIONS; @@ -122,10 +121,8 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> { thread_manager, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config.model = Some("gpt-5.2".to_string()); // Keep tool expectations stable when the default web_search mode changes. config @@ -236,10 +233,8 @@ async fn gpt_5_tools_without_apply_patch_append_apply_patch_instructions() -> an .await; let TestCodex { codex, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config .features .enable(Feature::CollaborationModes) @@ -320,10 +315,8 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests .await; let TestCodex { codex, config, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config .features .enable(Feature::CollaborationModes) @@ -420,10 +413,8 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an .await; let TestCodex { codex, config, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config .features .enable(Feature::CollaborationModes) @@ -714,10 +705,8 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Res .await; let TestCodex { codex, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config .features .enable(Feature::CollaborationModes) @@ -851,10 +840,8 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() -> a session_configured, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config .features .enable(Feature::CollaborationModes) @@ -994,10 +981,8 @@ async fn send_user_turn_with_changes_sends_environment_context() -> anyhow::Resu session_configured, .. } = test_codex() + .with_global_instructions("be consistent and helpful") .with_config(|config| { - config.user_instructions = Some(LoadedAgentsMd::from_text_for_testing( - "be consistent and helpful", - )); config .features .enable(Feature::CollaborationModes) diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 29b2d6c7d721..1b411fecc952 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -23,6 +23,7 @@ codex-config = { workspace = true } codex-core = { workspace = true } codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } +codex-home = { workspace = true } codex-login = { workspace = true } codex-protocol = { workspace = true } codex-utils-cli = { workspace = true } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 2f85b351435e..4ed4e7e4f6b0 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -6,7 +6,8 @@ use codex_core::StateDbHandle; use codex_core::ThreadManager; use codex_core::config::Config; use codex_exec_server::EnvironmentManager; -use codex_extension_api::empty_extension_registry; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_home::CodexHomeInstructionsContributor; use codex_login::AuthManager; use codex_login::default_client::USER_AGENT_SUFFIX; use codex_login::default_client::get_codex_user_agent; @@ -62,12 +63,16 @@ impl MessageProcessor { /*enable_codex_api_key_env*/ false, ) .await; + let mut extension_builder = ExtensionRegistryBuilder::::new(); + extension_builder.global_instructions_contributor(Arc::new( + CodexHomeInstructionsContributor::new(config.codex_home.clone()), + )); let thread_manager = Arc::new(ThreadManager::new( config.as_ref(), auth_manager, SessionSource::Mcp, environment_manager, - empty_extension_registry(), + Arc::new(extension_builder.build()), /*analytics_events_client*/ None, codex_core::thread_store_from_config(config.as_ref(), state_db.clone()), state_db.clone(), diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 47e0fc3911ad..feeb3dedab1d 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -16,6 +16,7 @@ use codex_core_api::AskForApproval; use codex_core_api::AuthCredentialsStoreMode; use codex_core_api::AuthManager; use codex_core_api::AutoCompactTokenLimitScope; +use codex_core_api::CodexHomeInstructionsContributor; use codex_core_api::CodexThread; use codex_core_api::Config; use codex_core_api::ConfigLayerStack; @@ -23,6 +24,7 @@ use codex_core_api::Constrained; use codex_core_api::EnvironmentManager; use codex_core_api::EventMsg; use codex_core_api::ExecServerRuntimePaths; +use codex_core_api::ExtensionRegistryBuilder; use codex_core_api::Features; use codex_core_api::GhostSnapshotConfig; use codex_core_api::History; @@ -54,7 +56,6 @@ use codex_core_api::UserInput; use codex_core_api::WebSearchMode; use codex_core_api::arg0_dispatch_or_else; use codex_core_api::built_in_model_providers; -use codex_core_api::empty_extension_registry; use codex_core_api::find_codex_home; use codex_core_api::init_state_db; use codex_core_api::item_event_to_server_notification; @@ -120,12 +121,16 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { .await?, ); let installation_id = resolve_installation_id(&config.codex_home).await?; + let mut extension_builder = ExtensionRegistryBuilder::::new(); + extension_builder.global_instructions_contributor(Arc::new( + CodexHomeInstructionsContributor::new(config.codex_home.clone()), + )); let thread_manager = ThreadManager::new( &config, auth_manager, SessionSource::Exec, environment_manager, - empty_extension_registry(), + Arc::new(extension_builder.build()), /*analytics_events_client*/ None, Arc::clone(&thread_store), state_db, @@ -184,7 +189,6 @@ fn new_config(model: Option, arg0_paths: Arg0DispatchPaths) -> anyhow::R enforce_residency: Constrained::allow_any(/*initial_value*/ None), hide_agent_reasoning: false, show_raw_agent_reasoning: false, - user_instructions: None, base_instructions: None, developer_instructions: None, guardian_policy_config: None,