Skip skill discovery when injecting items into initialized threads (#46546)

## Why

Recording response items in an initialized thread builds a turn context without starting a turn. Running host skill discovery for that context is unnecessary.

## What changed

Use a dedicated context build mode that skips host skill discovery for history injection. Preserve full discovery for initial context creation, startup prewarming, and execution. If compaction clears the reference context during preparation, rebuild a full context before initializing history.

## Testing

Add a regression test verifying that initial injection discovers skills, subsequent injection records items without discovery, and a real turn still discovers skills and sends the selected skill body and both injected messages to the model.

GitOrigin-RevId: 4a64e04eb0f21698f9b47160bfcfe5b6d6a68554
This commit is contained in:
Henry Levy
2026-09-19 01:03:32 +00:00
committed by copyberry
parent 05de935205
commit 7e0463b568
3 changed files with 171 additions and 16 deletions
+11 -1
View File
@@ -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
+41 -15
View File
@@ -359,9 +359,21 @@ pub struct TurnContext {
pub(crate) cyber_access_program: Option<CyberAccessProgram>,
}
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<TurnContext> {
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<TurnContext> {
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<TurnContext> {
self.new_default_turn_for(TurnContextBuildMode::InjectItems)
.await
}
async fn new_default_turn_for(&self, build_mode: TurnContextBuildMode) -> Arc<TurnContext> {
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
}
+119
View File
@@ -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<Vec<(String, SkillInvocationKind)>>);
@@ -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<Vec<u8>> = 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::<Vec<_>>();
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("<skill>\n<name>demo</name>") && 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(()));