diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 9bbbfe79db..0c54ced5cb 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -686,8 +686,18 @@ impl CodexThread { )); } - let turn_context = self.session.new_default_turn().await; + let had_reference_context = self.session.reference_context_item().await.is_some(); + let mut turn_context = if had_reference_context { + self.session.new_inject_items_context().await + } else { + self.session.new_default_turn().await + }; if self.session.reference_context_item().await.is_none() { + // Compaction can clear the reference while the recording context is built. + // Initial context must capture a step with a complete skills snapshot. + if had_reference_context { + turn_context = self.session.new_default_turn().await; + } // This history-only API runs without run_turn, so it owns its initial step. let step_context = self .session diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 22267d7881..163c57cfb5 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -359,9 +359,21 @@ pub struct TurnContext { pub(crate) cyber_access_program: Option, } -enum TurnMultiAgentRuntime { - ResolveAndStore, - Preview, +/// Selects which preparation is needed when building a turn context. +#[derive(Debug)] +enum TurnContextBuildMode { + /// Resolves and stores model/multi-agent metadata and performs normal skill discovery. + /// Used for execution and initial context creation. + Full, + + /// Prepares startup resources with normal skill discovery, without updating + /// shared model/multi-agent metadata. + StartupPrewarm, + + /// Resolves and stores model/multi-agent metadata but skips skill discovery. + /// Only for injecting items into an initialized thread; must not initialize + /// context or capture an execution step. + InjectItems, } impl TurnContext { @@ -1089,7 +1101,7 @@ impl Session { session_configuration, turn_environments, options, - TurnMultiAgentRuntime::ResolveAndStore, + TurnContextBuildMode::Full, self.git_enrichment_policy, ) .await @@ -1106,20 +1118,20 @@ impl Session { session_configuration, turn_environments, NewTurnContextOptions::default(), - TurnMultiAgentRuntime::Preview, + TurnContextBuildMode::StartupPrewarm, GitEnrichmentPolicy::Skip, ) .await } - #[instrument(name = "turn_context.build", level = "trace", skip_all)] + #[instrument(name = "turn_context.build", level = "trace", skip_all, fields(?build_mode))] async fn new_turn_context_from_configuration( &self, sub_id: String, session_configuration: SessionConfiguration, turn_environments: TurnEnvironmentSnapshot, options: NewTurnContextOptions, - multi_agent_runtime: TurnMultiAgentRuntime, + build_mode: TurnContextBuildMode, git_enrichment_policy: GitEnrichmentPolicy, ) -> Arc { let primary_turn_environment = turn_environments.primary(); @@ -1145,15 +1157,15 @@ impl Session { &session_configuration.model_info_overrides, ) .await; - let multi_agent_version = match multi_agent_runtime { - TurnMultiAgentRuntime::ResolveAndStore => { + let multi_agent_version = match build_mode { + TurnContextBuildMode::Full | TurnContextBuildMode::InjectItems => { // A background preview must not overwrite a newer turn's model metadata. self.services .thread_extension_data .insert(model_info.clone()); self.resolve_multi_agent_version_for_model(&model_info, &per_turn_config) } - TurnMultiAgentRuntime::Preview => per_turn_config.multi_agent_version_for_model( + TurnContextBuildMode::StartupPrewarm => per_turn_config.multi_agent_version_for_model( self.multi_agent_version() .or(model_info.multi_agent_version), ), @@ -1169,10 +1181,11 @@ impl Session { &plugin_outcome, per_turn_config.codex_home.as_path(), ); - let skills_snapshot = if per_turn_config - .features - .enabled(Feature::SkipHostSkillDiscovery) - && !self.services.extensions.requires_host_skill_discovery() + let skills_snapshot = if matches!(build_mode, TurnContextBuildMode::InjectItems) + || (per_turn_config + .features + .enabled(Feature::SkipHostSkillDiscovery) + && !self.services.extensions.requires_host_skill_discovery()) { // Executor and orchestrator catalogs are supplied independently of host skills. HostSkillsSnapshot::new(Arc::new(SkillLoadOutcome::default())) @@ -1298,13 +1311,26 @@ impl Session { /// Builds a context without starting work or changing the current environments. pub(crate) async fn new_default_turn(&self) -> Arc { + self.new_default_turn_for(TurnContextBuildMode::Full).await + } + + /// Captures current recording settings without discovering skills. + /// This context must not be used to capture or execute a step. + pub(crate) async fn new_inject_items_context(&self) -> Arc { + self.new_default_turn_for(TurnContextBuildMode::InjectItems) + .await + } + + async fn new_default_turn_for(&self, build_mode: TurnContextBuildMode) -> Arc { let session_configuration = self.default_turn_configuration().await; let turn_environments = self.services.turn_environments.snapshot().await; - self.new_turn_from_configuration( + self.new_turn_context_from_configuration( self.next_internal_sub_id(), session_configuration, turn_environments, NewTurnContextOptions::default(), + build_mode, + self.git_enrichment_policy, ) .await } diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 896ccc73ea..d44f29b6b8 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -15,8 +15,11 @@ use codex_extension_api::SkillInvocationKind; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; +use codex_protocol::models::ContentItem; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::user_input::UserInput; use codex_skills_extension::SkillsExtensionConfig; @@ -39,6 +42,9 @@ use core_test_support::test_codex::turn_permission_fields; use pretty_assertions::assert_eq; use std::sync::Arc; use std::sync::Mutex; +use tracing::Level; +use tracing_subscriber::fmt::format::FmtSpan; +use tracing_test::internal::MockWriter; #[derive(Default)] struct SkillInvocationRecorder(Mutex>); @@ -191,6 +197,119 @@ async fn user_turn_includes_skill_instructions() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "current_thread")] +async fn history_injection_skips_skill_discovery_after_initial_context() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "skill paths require matching host and executor path conventions" + ); + skip_if_no_network!(Ok(())); + + const SKILL_BODY: &str = "Instructions from the real demo skill."; + const FIRST_INSTRUCTIONS: &str = "First injected developer instructions."; + const SECOND_INSTRUCTIONS: &str = "Second injected developer instructions."; + + let buffer: &'static Mutex> = Box::leak(Box::new(Mutex::new(Vec::new()))); + let subscriber = tracing_subscriber::fmt() + .with_ansi(false) + .with_max_level(Level::TRACE) + .with_span_events(FmtSpan::NEW) + .with_writer(MockWriter::new(buffer)) + .finish(); + let _guard = tracing::subscriber::set_default(subscriber); + let take_logs = || String::from_utf8(std::mem::take(&mut *buffer.lock().unwrap())); + + let server = start_mock_server().await; + let mut builder = test_codex().with_workspace_setup(|cwd, fs| async move { + write_repo_skill(cwd, fs, "demo", "demo skill", SKILL_BODY).await + }); + let test = builder.build_with_auto_env(&server).await?; + let developer_message = |text: &str| ResponseItem::Message { + id: None, + role: "developer".to_string(), + content: vec![ContentItem::InputText { + text: text.to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }; + + // A fresh thread still needs full initial context before its first injected item. + buffer.lock().unwrap().clear(); + test.codex + .inject_response_items_for_turn(vec![developer_message(FIRST_INSTRUCTIONS)]) + .await?; + let logs = take_logs()?; + assert!( + logs.contains("skills_for_config"), + "initial injection must discover skills: {logs}" + ); + + test.codex + .inject_response_items_for_turn(vec![developer_message(SECOND_INSTRUCTIONS)]) + .await?; + let logs = take_logs()?; + assert!( + logs.contains("record_conversation_items"), + "initialized-history injection must still record the item: {logs}" + ); + assert!( + !logs.contains("skills_for_config"), + "initialized-history injection must skip skill discovery: {logs}" + ); + + let mock = mount_sse_once(&server, sse(vec![ev_completed("skill-response")])).await; + let skill_path = test + .config + .cwd + .join(".agents/skills/demo/SKILL.md") + .canonicalize() + .unwrap_or_else(|_| test.config.cwd.join(".agents/skills/demo/SKILL.md")) + .to_path_buf(); + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![ + UserInput::Text { + text: "please use $demo".to_string(), + text_elements: Vec::new(), + }, + UserInput::Skill { + name: "demo".to_string(), + path: skill_path, + }, + ])) + .await?; + core_test_support::wait_for_event(test.codex.as_ref(), |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + let logs = take_logs()?; + assert!( + logs.contains("skills_for_config"), + "the real turn must still discover skills: {logs}" + ); + + let request = mock.single_request(); + let developer_texts = request.message_input_texts("developer"); + let injected_texts = developer_texts + .iter() + .map(String::as_str) + .filter(|text| *text == FIRST_INSTRUCTIONS || *text == SECOND_INSTRUCTIONS) + .collect::>(); + assert_eq!( + injected_texts, + vec![FIRST_INSTRUCTIONS, SECOND_INSTRUCTIONS] + ); + let user_texts = request.message_input_texts("user"); + assert!( + user_texts.iter().any(|text| { + text.contains("\ndemo") && text.contains(SKILL_BODY) + }), + "the real skill body must reach the model: {user_texts:?}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn user_turn_selects_symlinked_skill_by_advertised_discovery_path() -> Result<()> { skip_if_no_network!(Ok(()));