mirror of
https://github.com/openai/codex.git
synced 2026-10-21 04:26:54 +00:00
Emit host skill budget warnings from world state (#35766)
## Why Host skill catalogs supplied through extension world state should report budget warnings only when their catalog update is actually rendered. ## What changed - Stop rebuilding an extension-owned host catalog in core. - Emit host catalog budget warnings when the corresponding world-state fragment is rendered, and deduplicate them with executor catalog warnings. - Cover full, shortened, and omitted host and executor catalogs, including repeated turns and resumed persisted snapshots. GitOrigin-RevId: ba28b6e337281b49ed4da2175c193fd4a6898ab7
This commit is contained in:
felixxia-oai
@@ -3376,22 +3376,18 @@ impl Session {
|
||||
{
|
||||
developer_sections.push(developer_instructions.to_string());
|
||||
}
|
||||
if turn_context.config.include_skill_instructions {
|
||||
let host_catalog_in_world_state = turn_context
|
||||
if turn_context.config.include_skill_instructions
|
||||
&& turn_context
|
||||
.extension_data
|
||||
.get::<HostSkillsCatalogInWorldState>()
|
||||
.is_some();
|
||||
let side_effects = if host_catalog_in_world_state {
|
||||
SkillRenderSideEffects::None
|
||||
} else {
|
||||
SkillRenderSideEffects::ThreadStart {
|
||||
session_telemetry: &self.services.session_telemetry,
|
||||
}
|
||||
};
|
||||
.is_none()
|
||||
{
|
||||
let available_skills = build_available_skills(
|
||||
turn_context.turn_skills.snapshot.outcome(),
|
||||
default_skill_metadata_budget(turn_context.model_info.context_window),
|
||||
side_effects,
|
||||
SkillRenderSideEffects::ThreadStart {
|
||||
session_telemetry: &self.services.session_telemetry,
|
||||
},
|
||||
);
|
||||
if let Some(available_skills) = available_skills {
|
||||
let warning_message = available_skills.warning_message.clone();
|
||||
@@ -3408,9 +3404,7 @@ impl Session {
|
||||
})
|
||||
.await;
|
||||
}
|
||||
if !host_catalog_in_world_state {
|
||||
developer_sections.push(skills_instructions.render());
|
||||
}
|
||||
developer_sections.push(skills_instructions.render());
|
||||
}
|
||||
}
|
||||
let loaded_plugins = self
|
||||
|
||||
@@ -1,11 +1,21 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_core::config::Config;
|
||||
use codex_extension_api::ExtensionEventSink;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_extension_api::ExtensionWarning;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_skills_extension::HostSkillProvider;
|
||||
use codex_skills_extension::SkillProvider;
|
||||
use codex_skills_extension::SkillProviderSource;
|
||||
use codex_skills_extension::SkillProviders;
|
||||
@@ -32,11 +42,17 @@ use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use toml::toml;
|
||||
|
||||
struct StaticSkillProvider {
|
||||
catalog: SkillCatalog,
|
||||
}
|
||||
|
||||
struct ExecutorSkillProvider {
|
||||
catalog: SkillCatalog,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
enum CapturedExtensionEvent {
|
||||
Event(Box<Event>),
|
||||
@@ -89,6 +105,321 @@ impl SkillProvider for StaticSkillProvider {
|
||||
}
|
||||
}
|
||||
|
||||
impl SkillProvider for ExecutorSkillProvider {
|
||||
fn list(&self, _query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> {
|
||||
Box::pin(async { Ok(self.catalog.clone()) })
|
||||
}
|
||||
|
||||
fn read(&self, _request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> {
|
||||
Box::pin(async {
|
||||
Err(SkillProviderError::new(
|
||||
"production-flow catalog test does not read skills",
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
fn search(&self, _request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult> {
|
||||
Box::pin(async { Ok(SkillSearchResult::default()) })
|
||||
}
|
||||
}
|
||||
|
||||
const FULL_CATALOG_CONTEXT_WINDOW: i64 = 40_000;
|
||||
const SHORTENING_CONTEXT_WINDOW: i64 = 12_000;
|
||||
const EXECUTOR_OMITTING_CONTEXT_WINDOW: i64 = 2_000;
|
||||
const HOST_OMITTING_CONTEXT_WINDOW: i64 = 2_000;
|
||||
const HOST_CATALOG: [(&str, &str); 4] = [
|
||||
(
|
||||
"host-alpha",
|
||||
"Host alpha reads local build files, checks repository conventions, and explains the safest small change before editing. It keeps host-only paths visible so the model can choose the right local instructions.",
|
||||
),
|
||||
(
|
||||
"host-beta",
|
||||
"Host beta reviews local test output, follows project-specific validation rules, and reports the smallest useful verification step. It is deliberately detailed enough to exercise description shortening.",
|
||||
),
|
||||
(
|
||||
"host-delta",
|
||||
"Host delta checks local dependency and formatting conventions before a change is finalized. It exists to make the host catalog explicit and to prove later host entries remain visible under pressure.",
|
||||
),
|
||||
(
|
||||
"host-gamma",
|
||||
"Host gamma inspects local configuration layers, keeps repository defaults intact, and points the model at the narrowest relevant file. Its description is long enough to share pressure with every other skill.",
|
||||
),
|
||||
];
|
||||
const EXECUTOR_CATALOG: [(&str, &str); 6] = [
|
||||
(
|
||||
"exec-alpha",
|
||||
"Executor alpha inspects environment-owned resources, resolves their exact package identifiers, and reads only the relevant instructions. It demonstrates the executor catalog rendering path under pressure.",
|
||||
),
|
||||
(
|
||||
"exec-beta",
|
||||
"Executor beta searches selected environment capabilities, keeps resource access bounded, and explains which remote instructions are available. It is intentionally long enough to be shortened fairly.",
|
||||
),
|
||||
(
|
||||
"exec-gamma",
|
||||
"Executor gamma reads environment-owned build metadata, preserves authority-aware locators, and avoids inventing filesystem paths. It makes the executor catalog large enough for shared allocation to matter.",
|
||||
),
|
||||
(
|
||||
"exec-delta",
|
||||
"Executor delta follows the selected environment workflow, loads only relevant resources, and reports remote constraints clearly. Its text should remain a visible prefix when descriptions are shortened.",
|
||||
),
|
||||
(
|
||||
"exec-epsilon",
|
||||
"Executor epsilon handles environment-specific validation steps, keeps reads bounded, and leaves unrelated capabilities alone. It proves later executor skills participate in round-robin shortening.",
|
||||
),
|
||||
(
|
||||
"exec-zeta",
|
||||
"Executor zeta resolves the final environment resource carefully, keeps package identifiers exact, and documents what remains available. It is the last explicit executor entry in this fixture.",
|
||||
),
|
||||
];
|
||||
|
||||
fn executor_catalog(skills: &[(&str, &str)]) -> SkillCatalog {
|
||||
SkillCatalog {
|
||||
entries: skills
|
||||
.iter()
|
||||
.map(|(name, description)| {
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(format!("test/{name}")),
|
||||
SkillAuthority::new(SkillSourceKind::Executor, "test"),
|
||||
*name,
|
||||
*description,
|
||||
SkillResourceId::new(format!("{name}/SKILL.md")),
|
||||
)
|
||||
.with_display_path(format!("skill://test/{name}/SKILL.md"))
|
||||
})
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn write_host_skills(codex_home: &std::path::Path, skills: &[(&str, &str)]) -> Result<()> {
|
||||
for (name, description) in skills {
|
||||
let skill_dir = codex_home.join("skills").join(name);
|
||||
std::fs::create_dir_all(&skill_dir)?;
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
format!("---\nname: {name}\ndescription: {description}\n---\n\n# body\n"),
|
||||
)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn catalog_extensions(
|
||||
executor_catalog: SkillCatalog,
|
||||
include_host_provider: bool,
|
||||
) -> (
|
||||
Arc<codex_extension_api::ExtensionRegistry<Config>>,
|
||||
std::sync::mpsc::Receiver<CapturedExtensionEvent>,
|
||||
) {
|
||||
let (event_tx, event_rx) = std::sync::mpsc::channel();
|
||||
let mut extensions =
|
||||
ExtensionRegistryBuilder::<Config>::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
|
||||
let mut providers =
|
||||
SkillProviders::new().with_executor_provider(Arc::new(ExecutorSkillProvider {
|
||||
catalog: executor_catalog,
|
||||
}));
|
||||
if include_host_provider {
|
||||
providers = providers.with_host_provider(Arc::new(HostSkillProvider::new()));
|
||||
}
|
||||
install_with_providers(&mut extensions, providers, |config: &Config| {
|
||||
SkillsExtensionConfig {
|
||||
include_instructions: config.include_skill_instructions,
|
||||
bundled_skills_enabled: false,
|
||||
orchestrator_skills_enabled: false,
|
||||
shadow_selection_enabled: false,
|
||||
}
|
||||
});
|
||||
(Arc::new(extensions.build()), event_rx)
|
||||
}
|
||||
|
||||
fn configure_catalog_test(config: &mut Config) {
|
||||
config.include_skill_instructions = true;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ExecutorCapabilityDiscovery)
|
||||
.expect("executor capability discovery should be configurable in tests");
|
||||
// A user layer also discovers the real `$HOME/.agents/skills`. Use a temporary system layer so
|
||||
// exact catalog and omission assertions only see the skills written under this test's home.
|
||||
let system_config_path = config.codex_home.join("config.toml");
|
||||
config.config_layer_stack = ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::System {
|
||||
file: system_config_path,
|
||||
},
|
||||
toml! { skills = { bundled = { enabled = false } } }.into(),
|
||||
)],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("skills test config should be valid");
|
||||
}
|
||||
|
||||
fn catalog_text<'a>(developer_texts: &'a [String], name_prefix: &str) -> &'a str {
|
||||
developer_texts
|
||||
.iter()
|
||||
.find(|text| text.contains(&format!("- {name_prefix}-")))
|
||||
.map(String::as_str)
|
||||
.unwrap_or_else(|| {
|
||||
panic!(
|
||||
"production request should include {name_prefix} skills, got {developer_texts:?}"
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn skill_lines<'a>(catalog_text: &'a str, name_prefix: &str) -> Vec<&'a str> {
|
||||
catalog_text
|
||||
.lines()
|
||||
.filter(|line| line.starts_with(&format!("- {name_prefix}-")))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn skill_names<'a>(skill_lines: &[&'a str]) -> Vec<&'a str> {
|
||||
skill_lines
|
||||
.iter()
|
||||
.map(|line| {
|
||||
line.strip_prefix("- ")
|
||||
.and_then(|line| line.split_once(": ").map(|(name, _)| name))
|
||||
.unwrap_or_else(|| panic!("skill line should contain a name separator: {line}"))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn rendered_description(skill_line: &str) -> &str {
|
||||
let (_, after_name) = skill_line
|
||||
.split_once(": ")
|
||||
.unwrap_or_else(|| panic!("skill line should contain a name separator: {skill_line}"));
|
||||
after_name
|
||||
.split_once(" (")
|
||||
.map_or("", |(description, _)| description)
|
||||
}
|
||||
|
||||
fn assert_full_descriptions(skill_lines: &[&str], expected: &[(&str, &str)]) {
|
||||
assert_eq!(
|
||||
skill_names(skill_lines),
|
||||
expected.iter().map(|(name, _)| *name).collect::<Vec<_>>()
|
||||
);
|
||||
for (skill_line, (_, expected_description)) in skill_lines.iter().zip(expected) {
|
||||
assert_eq!(rendered_description(skill_line), *expected_description);
|
||||
}
|
||||
}
|
||||
|
||||
fn assert_shortened_descriptions(skill_lines: &[&str], expected: &[(&str, &str)]) {
|
||||
assert_eq!(
|
||||
skill_names(skill_lines),
|
||||
expected.iter().map(|(name, _)| *name).collect::<Vec<_>>()
|
||||
);
|
||||
for (skill_line, (_, full_description)) in skill_lines.iter().zip(expected) {
|
||||
let description = rendered_description(skill_line);
|
||||
assert!(!description.is_empty());
|
||||
assert!(full_description.starts_with(description));
|
||||
assert!(description.chars().count() < full_description.chars().count());
|
||||
}
|
||||
}
|
||||
|
||||
fn metadata_cost(skill_lines: &[&str]) -> usize {
|
||||
skill_lines.iter().fold(0usize, |cost, line| {
|
||||
cost.saturating_add(approx_token_count(&format!("{line}\n")))
|
||||
})
|
||||
}
|
||||
|
||||
fn executor_omission_text(developer_texts: &[String]) -> &str {
|
||||
developer_texts
|
||||
.iter()
|
||||
.find(|text| text.contains("additional skills omitted from this bounded skills list"))
|
||||
.map(String::as_str)
|
||||
.unwrap_or_else(|| {
|
||||
panic!(
|
||||
"production request should include the executor omission marker, got {developer_texts:?}"
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
async fn rendered_catalogs(
|
||||
host_skills: &[(&str, &str)],
|
||||
executor_skills: &[(&str, &str)],
|
||||
context_window: i64,
|
||||
) -> Result<(Vec<String>, Vec<String>)> {
|
||||
rendered_catalogs_for_turns(
|
||||
host_skills,
|
||||
executor_skills,
|
||||
context_window,
|
||||
/*turn_count*/ 1,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn rendered_catalogs_for_turns(
|
||||
host_skills: &[(&str, &str)],
|
||||
executor_skills: &[(&str, &str)],
|
||||
context_window: i64,
|
||||
turn_count: usize,
|
||||
) -> Result<(Vec<String>, Vec<String>)> {
|
||||
let server = responses::start_mock_server().await;
|
||||
let response = responses::mount_sse_sequence(
|
||||
&server,
|
||||
(0..turn_count)
|
||||
.map(|index| {
|
||||
let response_id = format!("resp-{index}");
|
||||
sse(vec![
|
||||
ev_response_created(&response_id),
|
||||
ev_completed(&response_id),
|
||||
])
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
.await;
|
||||
let codex_home = Arc::new(TempDir::new()?);
|
||||
if !host_skills.is_empty() {
|
||||
write_host_skills(codex_home.path(), host_skills)?;
|
||||
}
|
||||
let (extensions, event_rx) =
|
||||
catalog_extensions(executor_catalog(executor_skills), !host_skills.is_empty());
|
||||
let mut builder = test_codex()
|
||||
.with_home(Arc::clone(&codex_home))
|
||||
.with_extensions(extensions)
|
||||
.with_model_info_override("gpt-5.5", move |model_info| {
|
||||
model_info.context_window = Some(context_window);
|
||||
model_info.max_context_window = None;
|
||||
})
|
||||
.with_config(configure_catalog_test);
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
|
||||
let mut client_warning_messages = Vec::new();
|
||||
for _ in 0..turn_count {
|
||||
test.codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "Inspect the available skills.".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: Default::default(),
|
||||
})
|
||||
.await?;
|
||||
loop {
|
||||
match core_test_support::wait_for_event(&test.codex, |_| true).await {
|
||||
EventMsg::Warning(warning) => client_warning_messages.push(warning.message),
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
let developer_texts = response
|
||||
.last_request()
|
||||
.expect("production turn should issue a responses request")
|
||||
.message_input_texts("developer");
|
||||
// Extension warnings are client-visible through the app-server event sink,
|
||||
// while core warnings are delivered through the TestCodex event stream.
|
||||
// Count both paths so duplicate warning ownership cannot hide in this test.
|
||||
client_warning_messages.extend(event_rx.try_iter().filter_map(|event| match event {
|
||||
CapturedExtensionEvent::Warning(warning) => Some(warning.message),
|
||||
CapturedExtensionEvent::Event(_) => None,
|
||||
}));
|
||||
let _codex_home_guard = codex_home;
|
||||
Ok((developer_texts, client_warning_messages))
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_scales_extension_catalog_from_resolved_model_window() -> Result<()> {
|
||||
let mut included_counts = Vec::new();
|
||||
@@ -195,6 +526,127 @@ async fn production_turn_scales_extension_catalog_from_resolved_model_window() -
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_shortens_host_only_catalog_with_the_full_budget() -> Result<()> {
|
||||
let (developer_texts, _) =
|
||||
rendered_catalogs(&HOST_CATALOG, &[], SHORTENING_CONTEXT_WINDOW).await?;
|
||||
let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host");
|
||||
|
||||
assert_shortened_descriptions(&host_lines, &HOST_CATALOG);
|
||||
assert!(metadata_cost(&host_lines) <= 240);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_shortens_executor_only_catalog_with_the_full_budget() -> Result<()> {
|
||||
let (developer_texts, _) =
|
||||
rendered_catalogs(&[], &EXECUTOR_CATALOG, SHORTENING_CONTEXT_WINDOW).await?;
|
||||
let executor_lines = skill_lines(catalog_text(&developer_texts, "exec"), "exec");
|
||||
|
||||
assert_shortened_descriptions(&executor_lines, &EXECUTOR_CATALOG);
|
||||
assert!(metadata_cost(&executor_lines) <= 240);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_keeps_full_host_only_catalog_when_it_fits() -> Result<()> {
|
||||
let (developer_texts, _) =
|
||||
rendered_catalogs(&HOST_CATALOG, &[], FULL_CATALOG_CONTEXT_WINDOW).await?;
|
||||
let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host");
|
||||
|
||||
assert_full_descriptions(&host_lines, &HOST_CATALOG);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_keeps_full_executor_only_catalog_when_it_fits() -> Result<()> {
|
||||
let (developer_texts, _) =
|
||||
rendered_catalogs(&[], &EXECUTOR_CATALOG, FULL_CATALOG_CONTEXT_WINDOW).await?;
|
||||
let executor_lines = skill_lines(catalog_text(&developer_texts, "exec"), "exec");
|
||||
|
||||
assert_full_descriptions(&executor_lines, &EXECUTOR_CATALOG);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_keeps_full_host_and_executor_catalogs_when_they_fit() -> Result<()> {
|
||||
let (developer_texts, _) = rendered_catalogs(
|
||||
&HOST_CATALOG,
|
||||
&EXECUTOR_CATALOG,
|
||||
FULL_CATALOG_CONTEXT_WINDOW,
|
||||
)
|
||||
.await?;
|
||||
let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host");
|
||||
let executor_lines = skill_lines(catalog_text(&developer_texts, "exec"), "exec");
|
||||
|
||||
assert_full_descriptions(&host_lines, &HOST_CATALOG);
|
||||
assert_full_descriptions(&executor_lines, &EXECUTOR_CATALOG);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_omits_host_skills_under_extreme_host_only_pressure() -> Result<()> {
|
||||
let (developer_texts, warning_messages) =
|
||||
rendered_catalogs(&HOST_CATALOG, &[], HOST_OMITTING_CONTEXT_WINDOW).await?;
|
||||
let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host");
|
||||
|
||||
assert_eq!(
|
||||
skill_names(&host_lines),
|
||||
vec!["host-alpha", "host-beta", "host-delta"]
|
||||
);
|
||||
let expected_warning = "Exceeded skills context budget of 2%. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list.";
|
||||
assert_eq!(
|
||||
warning_messages
|
||||
.iter()
|
||||
.filter(|message| message.as_str() == expected_warning)
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn successive_turns_do_not_repeat_unchanged_host_budget_warning() -> Result<()> {
|
||||
let (_, warning_messages) = rendered_catalogs_for_turns(
|
||||
&HOST_CATALOG,
|
||||
&[],
|
||||
HOST_OMITTING_CONTEXT_WINDOW,
|
||||
/*turn_count*/ 2,
|
||||
)
|
||||
.await?;
|
||||
let expected_warning = "Exceeded skills context budget of 2%. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list.";
|
||||
|
||||
assert_eq!(
|
||||
warning_messages
|
||||
.iter()
|
||||
.filter(|message| message.as_str() == expected_warning)
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_omits_executor_skills_under_extreme_executor_only_pressure() -> Result<()>
|
||||
{
|
||||
let (developer_texts, _) =
|
||||
rendered_catalogs(&[], &EXECUTOR_CATALOG, EXECUTOR_OMITTING_CONTEXT_WINDOW).await?;
|
||||
let executor_text = executor_omission_text(&developer_texts);
|
||||
let executor_lines = skill_lines(executor_text, "exec");
|
||||
|
||||
assert_eq!(skill_names(&executor_lines), vec!["exec-alpha"]);
|
||||
assert!(executor_text.contains("- 5 additional skills omitted from this bounded skills list."));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_fairly_shortens_extension_catalog_descriptions() -> Result<()> {
|
||||
let server = responses::start_mock_server().await;
|
||||
|
||||
@@ -63,6 +63,7 @@ use crate::state::SkillsThreadState;
|
||||
use crate::state::SkillsTurnState;
|
||||
use crate::tools::skill_tools;
|
||||
use crate::warnings::bounded_warnings;
|
||||
use crate::world_state::HostSkillsWarningEmitter;
|
||||
use crate::world_state::executor_skills_world_state_section;
|
||||
use crate::world_state::host_skills_world_state_section;
|
||||
|
||||
@@ -270,31 +271,46 @@ where
|
||||
} else {
|
||||
RenderedCatalog::default()
|
||||
};
|
||||
if let Some(message) = rendered.warning_message
|
||||
&& input
|
||||
.turn_store
|
||||
.get_or_init(EmittedCatalogBudgetWarnings::default)
|
||||
.insert(&message)
|
||||
{
|
||||
self.emit_warning(input.thread_store.level_id(), Some(input.turn_id), message);
|
||||
}
|
||||
let warning_message = rendered.warning_message;
|
||||
let executor_body = rendered.fragment.map(|fragment| fragment.body());
|
||||
let mut sections = vec![executor_skills_world_state_section(
|
||||
executor_body,
|
||||
config.include_instructions,
|
||||
)];
|
||||
let emitted_warnings = input
|
||||
.turn_store
|
||||
.get_or_init(EmittedCatalogBudgetWarnings::default);
|
||||
if let Some(host_snapshot) = input.turn_store.get::<HostSkillsSnapshot>()
|
||||
&& self.providers.has_host_provider()
|
||||
{
|
||||
input.turn_store.insert(HostSkillsCatalogInWorldState);
|
||||
let event_sink = Arc::clone(&self.event_sink);
|
||||
let emitted_warnings = Arc::clone(&emitted_warnings);
|
||||
let thread_id = input.thread_store.level_id().to_string();
|
||||
let turn_id = input.turn_id.to_string();
|
||||
let warning_emitter: HostSkillsWarningEmitter = Arc::new(move |message| {
|
||||
if emitted_warnings.insert(&message) {
|
||||
event_sink.emit_warning(ExtensionWarning {
|
||||
thread_id: thread_id.clone(),
|
||||
turn_id: Some(turn_id.clone()),
|
||||
message,
|
||||
});
|
||||
}
|
||||
});
|
||||
sections.push(host_skills_world_state_section(
|
||||
&host_snapshot,
|
||||
config.include_instructions,
|
||||
include_usage,
|
||||
metadata_budget,
|
||||
extension_metrics,
|
||||
warning_emitter,
|
||||
));
|
||||
}
|
||||
if let Some(message) = warning_message
|
||||
&& emitted_warnings.insert(&message)
|
||||
{
|
||||
self.emit_warning(input.thread_store.level_id(), Some(input.turn_id), message);
|
||||
}
|
||||
sections
|
||||
})
|
||||
}
|
||||
|
||||
@@ -26,6 +26,8 @@ const NO_HOST_SKILLS_BODY: &str =
|
||||
"\n## Host skills update\nNo host skills are currently available.\n";
|
||||
const HIDDEN_HOST_SKILLS_BODY: &str = "\n## Host skills update\nHost skills are not listed automatically. Explicit skill mentions can still be resolved when available.\n";
|
||||
|
||||
pub(crate) type HostSkillsWarningEmitter = Arc<dyn Fn(String) + Send + Sync>;
|
||||
|
||||
pub(crate) fn executor_skills_world_state_section(
|
||||
body: Option<String>,
|
||||
include_instructions: bool,
|
||||
@@ -84,6 +86,7 @@ pub(crate) fn host_skills_world_state_section(
|
||||
include_skills_usage_instructions: bool,
|
||||
metadata_budget: SkillMetadataBudget,
|
||||
extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||
warning_emitter: HostSkillsWarningEmitter,
|
||||
) -> WorldStateSectionContribution {
|
||||
let outcome = host_snapshot.outcome();
|
||||
let metadata_budget = match metadata_budget {
|
||||
@@ -97,6 +100,9 @@ pub(crate) fn host_skills_world_state_section(
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let warning_message = available
|
||||
.as_ref()
|
||||
.and_then(|available| available.warning_message.clone());
|
||||
let render_metrics = include_instructions.then(|| {
|
||||
available
|
||||
.as_ref()
|
||||
@@ -118,6 +124,24 @@ pub(crate) fn host_skills_world_state_section(
|
||||
)
|
||||
.body()
|
||||
});
|
||||
rendered_host_skills_world_state_section(
|
||||
body,
|
||||
include_instructions,
|
||||
render_metrics,
|
||||
extension_metrics,
|
||||
warning_message,
|
||||
warning_emitter,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn rendered_host_skills_world_state_section(
|
||||
body: Option<String>,
|
||||
include_instructions: bool,
|
||||
render_metrics: Option<(usize, usize, usize, usize)>,
|
||||
extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||
warning_message: Option<String>,
|
||||
warning_emitter: HostSkillsWarningEmitter,
|
||||
) -> WorldStateSectionContribution {
|
||||
let snapshot = json!({
|
||||
"body": body,
|
||||
"includeInstructions": include_instructions,
|
||||
@@ -159,6 +183,9 @@ pub(crate) fn host_skills_world_state_section(
|
||||
None if !include_instructions => HIDDEN_HOST_SKILLS_BODY,
|
||||
None => NO_HOST_SKILLS_BODY,
|
||||
};
|
||||
if let Some(message) = warning_message.as_ref() {
|
||||
warning_emitter(message.clone());
|
||||
}
|
||||
Some(RenderedWorldStateFragment::new(
|
||||
"developer",
|
||||
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG),
|
||||
|
||||
@@ -304,6 +304,112 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persisted_host_snapshot_deduplicates_warning_after_reinitialization() -> TestResult {
|
||||
let (event_tx, event_rx) = std::sync::mpsc::channel();
|
||||
let mut builder =
|
||||
ExtensionRegistryBuilder::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
|
||||
install(&mut builder, skills_extension_config);
|
||||
let registry = builder.build();
|
||||
let session_store = ExtensionData::new("session");
|
||||
let config = default_config();
|
||||
let skill_path = AbsolutePathBuf::try_from(test_codex_home().join("skills/demo/SKILL.md"))?;
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
outcome.skills.push(SkillMetadata {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill.".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
});
|
||||
let host_snapshot = HostSkillsSnapshot::new(Arc::new(outcome));
|
||||
|
||||
let thread_store = ExtensionData::new("thread");
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
session_source: &SessionSource::Cli,
|
||||
persistent_thread_state_available: true,
|
||||
environments: &[],
|
||||
mcp_resource_client: None,
|
||||
extension_metrics: None,
|
||||
session_store: &session_store,
|
||||
thread_store: &thread_store,
|
||||
})
|
||||
.await;
|
||||
let mut model_info = model_info_from_slug("test-model");
|
||||
model_info.context_window = Some(50);
|
||||
thread_store.insert(model_info);
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
turn_store.insert(host_snapshot.clone());
|
||||
let sections = registry.context_contributors()[0]
|
||||
.contribute_world_state(WorldStateContributionInput {
|
||||
thread_id: codex_protocol::ThreadId::new(),
|
||||
turn_id: "turn-1",
|
||||
environments: &[],
|
||||
ready_selected_capability_roots: &[],
|
||||
executor_capability_discovery: None,
|
||||
extension_metrics: None,
|
||||
session_store: &session_store,
|
||||
thread_store: &thread_store,
|
||||
turn_store: &turn_store,
|
||||
})
|
||||
.await;
|
||||
let published_snapshot = sections[1].snapshot().clone();
|
||||
assert!(
|
||||
sections[1]
|
||||
.render_diff(PreviousWorldStateSection::Absent)
|
||||
.is_some()
|
||||
);
|
||||
event_rx.try_recv()?.into_warning();
|
||||
assert!(event_rx.try_recv().is_err());
|
||||
|
||||
let resumed_thread_store = ExtensionData::new("thread");
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
session_source: &SessionSource::Cli,
|
||||
persistent_thread_state_available: true,
|
||||
environments: &[],
|
||||
mcp_resource_client: None,
|
||||
extension_metrics: None,
|
||||
session_store: &session_store,
|
||||
thread_store: &resumed_thread_store,
|
||||
})
|
||||
.await;
|
||||
let mut model_info = model_info_from_slug("test-model");
|
||||
model_info.context_window = Some(50);
|
||||
resumed_thread_store.insert(model_info);
|
||||
let resumed_turn_store = ExtensionData::new("turn-2");
|
||||
resumed_turn_store.insert(host_snapshot);
|
||||
let sections = registry.context_contributors()[0]
|
||||
.contribute_world_state(WorldStateContributionInput {
|
||||
thread_id: codex_protocol::ThreadId::new(),
|
||||
turn_id: "turn-2",
|
||||
environments: &[],
|
||||
ready_selected_capability_roots: &[],
|
||||
executor_capability_discovery: None,
|
||||
extension_metrics: None,
|
||||
session_store: &session_store,
|
||||
thread_store: &resumed_thread_store,
|
||||
turn_store: &resumed_turn_store,
|
||||
})
|
||||
.await;
|
||||
assert!(
|
||||
sections[1]
|
||||
.render_diff(PreviousWorldStateSection::Known(&published_snapshot))
|
||||
.is_none()
|
||||
);
|
||||
assert!(event_rx.try_recv().is_err());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn nonempty_executor_empty_host_records_catalog_metrics() -> TestResult {
|
||||
let executor_provider = Arc::new(StaticSkillProvider {
|
||||
|
||||
Reference in New Issue
Block a user