Delete Codex core skills crate

This commit is contained in:
Meta Vens 2026-05-14 02:28:00 +01:00
commit 67b2c3d176
39 changed files with 15 additions and 6850 deletions

View file

@ -902,9 +902,13 @@ than repaired into new compatibility lies.
The live skill mention channel is also gone. `UserInput::Skill`, the app-server
schema branch, TUI skill popup, `skills_all` cache, composer skill mention API,
skill insertion path, `skill://` / `SKILL.md` history decoding, and stale skill
popup snapshot were deleted. Live structured user-input mentions are MCP-shaped
only; MCP remains JSON-RPC at its protocol edge, not a reason to keep Codex
skills alive inside the organ.
popup snapshot were deleted. The deeper `codex-core-skills` crate is now deleted
from the workspace too, along with the core `skills` re-export shim, no-op
`skills_watcher`, session `SkillsManager` service, and per-turn
`TurnSkillsContext`; core keeps only a local legacy `<skill>` fragment recognizer
for old history classification. Live structured user-input mentions are
MCP-shaped only; MCP remains JSON-RPC at its protocol edge, not a reason to keep
Codex skills alive inside the organ.
That husk has now been cut from `codex-core`. The root core crate no longer
exports `plugins`, no longer depends on `codex-core-plugins` or `codex-plugin`,

Binary file not shown.

File diff suppressed because one or more lines are too long

49
vendor/codex/codex-rs/Cargo.lock generated vendored
View file

@ -2345,7 +2345,6 @@ dependencies = [
"codex-async-utils",
"codex-code-mode",
"codex-config",
"codex-core-skills",
"codex-exec-server",
"codex-execpolicy",
"codex-features",
@ -2446,34 +2445,6 @@ dependencies = [
"zstd 0.13.3",
]
[[package]]
name = "codex-core-skills"
version = "0.0.0"
dependencies = [
"anyhow",
"codex-analytics",
"codex-app-server-protocol",
"codex-config",
"codex-exec-server",
"codex-login",
"codex-otel",
"codex-protocol",
"codex-utils-absolute-path",
"codex-utils-output-truncation",
"dirs",
"dunce",
"pretty_assertions",
"serde",
"serde_json",
"serde_yaml",
"shlex",
"tempfile",
"tokio",
"toml 0.9.11+spec-1.1.0",
"tracing",
"zip 2.4.2",
]
[[package]]
name = "codex-debug-client"
version = "0.0.0"
@ -3367,7 +3338,6 @@ dependencies = [
"codex-cli",
"codex-cloud-requirements",
"codex-config",
"codex-core-skills",
"codex-exec-server",
"codex-features",
"codex-feedback",
@ -11680,19 +11650,6 @@ dependencies = [
"syn 2.0.114",
]
[[package]]
name = "serde_yaml"
version = "0.9.34+deprecated"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47"
dependencies = [
"indexmap 2.13.0",
"itoa",
"ryu",
"serde",
"unsafe-libyaml",
]
[[package]]
name = "serial2"
version = "0.2.33"
@ -13562,12 +13519,6 @@ dependencies = [
"subtle",
]
[[package]]
name = "unsafe-libyaml"
version = "0.2.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861"
[[package]]
name = "untrusted"
version = "0.7.1"

View file

@ -29,7 +29,6 @@ members = [
"shell-command",
"shell-escalation",
"core",
"core-skills",
"hooks",
"secrets",
"exec",
@ -129,7 +128,6 @@ codex-cloud-tasks-mock-client = { path = "cloud-tasks-mock-client" }
codex-code-mode = { path = "code-mode" }
codex-config = { path = "config" }
codex-core = { path = "core" }
codex-core-skills = { path = "core-skills" }
codex-device-key = { path = "device-key" }
codex-exec = { path = "exec" }
codex-exec-server = { path = "exec-server" }

View file

@ -1,15 +0,0 @@
load("//:defs.bzl", "codex_rust_crate")
codex_rust_crate(
name = "core-skills",
crate_name = "codex_core_skills",
compile_data = glob(
include = ["**"],
exclude = [
"**/* *",
"BUILD.bazel",
"Cargo.toml",
],
allow_empty = True,
),
)

View file

@ -1,39 +0,0 @@
[package]
edition.workspace = true
license.workspace = true
name = "codex-core-skills"
version.workspace = true
[lib]
doctest = false
name = "codex_core_skills"
path = "src/lib.rs"
[lints]
workspace = true
[dependencies]
anyhow = { workspace = true }
codex-analytics = { workspace = true }
codex-app-server-protocol = { workspace = true }
codex-config = { workspace = true }
codex-exec-server = { workspace = true }
codex-login = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-output-truncation = { workspace = true }
dirs = { workspace = true }
dunce = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
shlex = { workspace = true }
tokio = { workspace = true, features = ["fs", "macros", "rt"] }
toml = { workspace = true }
tracing = { workspace = true }
zip = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }

View file

@ -1,128 +0,0 @@
use std::collections::HashSet;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::SkillConfig;
use codex_config::SkillsConfig;
use codex_utils_absolute_path::AbsolutePathBuf;
use tracing::warn;
use crate::SkillMetadata;
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum SkillConfigRuleSelector {
Name(String),
Path(AbsolutePathBuf),
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct SkillConfigRule {
pub selector: SkillConfigRuleSelector,
pub enabled: bool,
}
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
pub struct SkillConfigRules {
pub entries: Vec<SkillConfigRule>,
}
pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> SkillConfigRules {
let mut entries = Vec::new();
for layer in config_layer_stack.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ true,
) {
if !matches!(
layer.name,
ConfigLayerSource::User { .. } | ConfigLayerSource::SessionFlags
) {
continue;
}
let Some(skills_value) = layer.config.get("skills") else {
continue;
};
let skills: SkillsConfig = match skills_value.clone().try_into() {
Ok(skills) => skills,
Err(err) => {
warn!("invalid skills config: {err}");
continue;
}
};
for entry in skills.config {
let Some(selector) = skill_config_rule_selector(&entry) else {
continue;
};
// Preserve layer order so a later name selector can override an earlier path selector
// for the same loaded skill.
entries.retain(|entry: &SkillConfigRule| entry.selector != selector);
entries.push(SkillConfigRule {
selector,
enabled: entry.enabled,
});
}
}
SkillConfigRules { entries }
}
pub fn resolve_disabled_skill_paths(
skills: &[SkillMetadata],
rules: &SkillConfigRules,
) -> HashSet<AbsolutePathBuf> {
let mut disabled_paths = HashSet::new();
for entry in &rules.entries {
match &entry.selector {
SkillConfigRuleSelector::Path(path) => {
if entry.enabled {
disabled_paths.remove(path);
} else {
disabled_paths.insert(path.clone());
}
}
SkillConfigRuleSelector::Name(name) => {
for path in skills
.iter()
.filter(|skill| skill.name == *name)
.map(|skill| skill.path_to_skills_md.clone())
{
if entry.enabled {
disabled_paths.remove(&path);
} else {
disabled_paths.insert(path);
}
}
}
}
}
disabled_paths
}
fn skill_config_rule_selector(entry: &SkillConfig) -> Option<SkillConfigRuleSelector> {
match (entry.path.as_ref(), entry.name.as_deref()) {
(Some(path), None) => Some(SkillConfigRuleSelector::Path(
path.canonicalize().unwrap_or_else(|_| path.clone()),
)),
(None, Some(name)) => {
let name = name.trim();
if name.is_empty() {
warn!("ignoring empty skills.config name override");
None
} else {
Some(SkillConfigRuleSelector::Name(name.to_string()))
}
}
(Some(_), Some(_)) => {
warn!("ignoring skills.config entry with both path and name selectors");
None
}
(None, None) => {
warn!("ignoring skills.config entry without a path or name selector");
None
}
}
}

View file

@ -1,30 +0,0 @@
use crate::SkillMetadata;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillDependencyInfo {
pub skill_name: String,
pub name: String,
pub description: Option<String>,
}
pub fn collect_env_var_dependencies(
mentioned_skills: &[SkillMetadata],
) -> Vec<SkillDependencyInfo> {
let mut dependencies = Vec::new();
for skill in mentioned_skills {
let Some(skill_dependencies) = &skill.dependencies else {
continue;
};
for tool in &skill_dependencies.tools {
if tool.r#type != "env_var" || tool.value.is_empty() {
continue;
}
dependencies.push(SkillDependencyInfo {
skill_name: skill.name.clone(),
name: tool.value.clone(),
description: tool.description.clone(),
});
}
}
dependencies
}

View file

@ -1,455 +0,0 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use crate::SkillLoadOutcome;
use crate::SkillMetadata;
use crate::build_skill_name_counts;
use codex_analytics::AnalyticsEventsClient;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::TrackEventsContext;
use codex_exec_server::LOCAL_FS;
use codex_otel::SessionTelemetry;
use codex_utils_absolute_path::AbsolutePathBuf;
const TOOL_MENTION_SIGIL: char = '$';
#[derive(Debug, Default)]
pub struct SkillInjections {
pub items: Vec<SkillInjection>,
pub warnings: Vec<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillInjection {
pub name: String,
pub path: String,
pub contents: String,
}
pub async fn build_skill_injections(
mentioned_skills: &[SkillMetadata],
loaded_skills: Option<&SkillLoadOutcome>,
otel: Option<&SessionTelemetry>,
analytics_client: &AnalyticsEventsClient,
tracking: TrackEventsContext,
) -> SkillInjections {
if mentioned_skills.is_empty() {
return SkillInjections::default();
}
let mut result = SkillInjections {
items: Vec::with_capacity(mentioned_skills.len()),
warnings: Vec::new(),
};
let mut invocations = Vec::new();
for skill in mentioned_skills {
let fs = loaded_skills
.and_then(|outcome| outcome.file_system_for_skill(skill))
.unwrap_or_else(|| Arc::clone(&LOCAL_FS));
match fs
.read_file_text(&skill.path_to_skills_md, /*sandbox*/ None)
.await
{
Ok(contents) => {
emit_skill_injected_metric(otel, skill, "ok");
invocations.push(SkillInvocation {
skill_name: skill.name.clone(),
skill_scope: skill.scope,
skill_path: skill.path_to_skills_md.to_path_buf(),
invocation_type: InvocationType::Explicit,
});
result.items.push(SkillInjection {
name: skill.name.clone(),
path: skill.path_to_skills_md.to_string_lossy().into_owned(),
contents,
});
}
Err(err) => {
emit_skill_injected_metric(otel, skill, "error");
let message = format!(
"Failed to load skill {name} at {path}: {err:#}",
name = skill.name,
path = skill.path_to_skills_md.display()
);
result.warnings.push(message);
}
}
}
analytics_client.track_skill_invocations(tracking, invocations);
result
}
fn emit_skill_injected_metric(
otel: Option<&SessionTelemetry>,
skill: &SkillMetadata,
status: &str,
) {
let Some(otel) = otel else {
return;
};
otel.counter(
"codex.skill.injected",
/*inc*/ 1,
&[("status", status), ("skill", skill.name.as_str())],
);
}
/// Collect explicitly mentioned skills from text mentions.
///
/// Text inputs are scanned to extract `$skill-name` tokens, and we iterate `skills` in
/// their existing order to preserve prior ordering semantics.
/// Explicit links are resolved by path and plain names are only used when the match
/// is unambiguous.
///
/// Complexity: `O(T + N * S)` time, `O(S + M)` space, where:
/// `S` = number of skills, `T` = total text length, `N` = number of text inputs,
/// and `M` = max mentions parsed from a single text input.
pub fn collect_explicit_skill_mentions(
inputs: &[codex_protocol::user_input::UserInput],
skills: &[SkillMetadata],
disabled_paths: &HashSet<AbsolutePathBuf>,
connector_slug_counts: &HashMap<String, usize>,
) -> Vec<SkillMetadata> {
let skill_name_counts = build_skill_name_counts(skills, disabled_paths).0;
let selection_context = SkillSelectionContext {
skills,
disabled_paths,
skill_name_counts: &skill_name_counts,
connector_slug_counts,
};
let mut selected: Vec<SkillMetadata> = Vec::new();
let mut seen_names: HashSet<String> = HashSet::new();
let mut seen_paths: HashSet<AbsolutePathBuf> = HashSet::new();
for input in inputs {
if let codex_protocol::user_input::UserInput::Text { text, .. } = input {
let mentioned_names = extract_tool_mentions(text);
select_skills_from_mentions(
&selection_context,
&mentioned_names,
&mut seen_names,
&mut seen_paths,
&mut selected,
);
}
}
selected
}
struct SkillSelectionContext<'a> {
skills: &'a [SkillMetadata],
disabled_paths: &'a HashSet<AbsolutePathBuf>,
skill_name_counts: &'a HashMap<String, usize>,
connector_slug_counts: &'a HashMap<String, usize>,
}
pub struct ToolMentions<'a> {
names: HashSet<&'a str>,
paths: HashSet<&'a str>,
plain_names: HashSet<&'a str>,
}
impl<'a> ToolMentions<'a> {
fn is_empty(&self) -> bool {
self.names.is_empty() && self.paths.is_empty()
}
pub fn plain_names(&self) -> impl Iterator<Item = &'a str> + '_ {
self.plain_names.iter().copied()
}
pub fn paths(&self) -> impl Iterator<Item = &'a str> + '_ {
self.paths.iter().copied()
}
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum ToolMentionKind {
Mcp,
Skill,
Other,
}
const MCP_PATH_PREFIX: &str = "mcp://";
const SKILL_PATH_PREFIX: &str = "skill://";
const SKILL_FILENAME: &str = "SKILL.md";
pub fn tool_kind_for_path(path: &str) -> ToolMentionKind {
if path.starts_with(MCP_PATH_PREFIX) {
ToolMentionKind::Mcp
} else if path.starts_with(SKILL_PATH_PREFIX) || is_skill_filename(path) {
ToolMentionKind::Skill
} else {
ToolMentionKind::Other
}
}
fn is_skill_filename(path: &str) -> bool {
let file_name = path.rsplit(['/', '\\']).next().unwrap_or(path);
file_name.eq_ignore_ascii_case(SKILL_FILENAME)
}
pub(crate) fn normalize_skill_path(path: &str) -> &str {
path.strip_prefix(SKILL_PATH_PREFIX).unwrap_or(path)
}
/// Extract `$tool-name` mentions from a single text input.
///
/// Supports explicit resource links in the form `[$tool-name](resource path)`. When a
/// resource path is present, it is captured for exact path matching while also tracking
/// the name for fallback matching.
pub fn extract_tool_mentions(text: &str) -> ToolMentions<'_> {
extract_tool_mentions_with_sigil(text, TOOL_MENTION_SIGIL)
}
pub fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions<'_> {
let text_bytes = text.as_bytes();
let mut mentioned_names: HashSet<&str> = HashSet::new();
let mut mentioned_paths: HashSet<&str> = HashSet::new();
let mut plain_names: HashSet<&str> = HashSet::new();
let mut index = 0;
while index < text_bytes.len() {
let byte = text_bytes[index];
if byte == b'['
&& let Some((name, path, end_index)) =
parse_linked_tool_mention(text, text_bytes, index, sigil)
{
if !is_common_env_var(name) {
if !matches!(tool_kind_for_path(path), ToolMentionKind::Mcp) {
mentioned_names.insert(name);
}
mentioned_paths.insert(path);
}
index = end_index;
continue;
}
if byte != sigil as u8 {
index += 1;
continue;
}
let name_start = index + 1;
let Some(first_name_byte) = text_bytes.get(name_start) else {
index += 1;
continue;
};
if !is_mention_name_char(*first_name_byte) {
index += 1;
continue;
}
let mut name_end = name_start + 1;
while let Some(next_byte) = text_bytes.get(name_end)
&& is_mention_name_char(*next_byte)
{
name_end += 1;
}
let name = &text[name_start..name_end];
if !is_common_env_var(name) {
mentioned_names.insert(name);
plain_names.insert(name);
}
index = name_end;
}
ToolMentions {
names: mentioned_names,
paths: mentioned_paths,
plain_names,
}
}
/// Select mentioned skills while preserving the order of `skills`.
fn select_skills_from_mentions(
selection_context: &SkillSelectionContext<'_>,
mentions: &ToolMentions<'_>,
seen_names: &mut HashSet<String>,
seen_paths: &mut HashSet<AbsolutePathBuf>,
selected: &mut Vec<SkillMetadata>,
) {
if mentions.is_empty() {
return;
}
let mention_skill_paths: HashSet<&str> = mentions
.paths()
.filter(|path| !matches!(tool_kind_for_path(path), ToolMentionKind::Mcp))
.map(normalize_skill_path)
.collect();
for skill in selection_context.skills {
if selection_context
.disabled_paths
.contains(&skill.path_to_skills_md)
|| seen_paths.contains(&skill.path_to_skills_md)
{
continue;
}
let path_str = skill.path_to_skills_md.to_string_lossy();
if mention_skill_paths.contains(path_str.as_ref()) {
seen_paths.insert(skill.path_to_skills_md.clone());
seen_names.insert(skill.name.clone());
selected.push(skill.clone());
}
}
for skill in selection_context.skills {
if selection_context
.disabled_paths
.contains(&skill.path_to_skills_md)
|| seen_paths.contains(&skill.path_to_skills_md)
{
continue;
}
if !mentions.plain_names.contains(skill.name.as_str()) {
continue;
}
let skill_count = selection_context
.skill_name_counts
.get(skill.name.as_str())
.copied()
.unwrap_or(0);
let connector_count = selection_context
.connector_slug_counts
.get(&skill.name.to_ascii_lowercase())
.copied()
.unwrap_or(0);
if skill_count != 1 || connector_count != 0 {
continue;
}
if seen_names.insert(skill.name.clone()) {
seen_paths.insert(skill.path_to_skills_md.clone());
selected.push(skill.clone());
}
}
}
fn parse_linked_tool_mention<'a>(
text: &'a str,
text_bytes: &[u8],
start: usize,
sigil: char,
) -> Option<(&'a str, &'a str, usize)> {
let sigil_index = start + 1;
if text_bytes.get(sigil_index) != Some(&(sigil as u8)) {
return None;
}
let name_start = sigil_index + 1;
let first_name_byte = text_bytes.get(name_start)?;
if !is_mention_name_char(*first_name_byte) {
return None;
}
let mut name_end = name_start + 1;
while let Some(next_byte) = text_bytes.get(name_end)
&& is_mention_name_char(*next_byte)
{
name_end += 1;
}
if text_bytes.get(name_end) != Some(&b']') {
return None;
}
let mut path_start = name_end + 1;
while let Some(next_byte) = text_bytes.get(path_start)
&& next_byte.is_ascii_whitespace()
{
path_start += 1;
}
if text_bytes.get(path_start) != Some(&b'(') {
return None;
}
let mut path_end = path_start + 1;
while let Some(next_byte) = text_bytes.get(path_end)
&& *next_byte != b')'
{
path_end += 1;
}
if text_bytes.get(path_end) != Some(&b')') {
return None;
}
let path = text[path_start + 1..path_end].trim();
if path.is_empty() {
return None;
}
let name = &text[name_start..name_end];
Some((name, path, path_end + 1))
}
fn is_common_env_var(name: &str) -> bool {
let upper = name.to_ascii_uppercase();
matches!(
upper.as_str(),
"PATH"
| "HOME"
| "USER"
| "SHELL"
| "PWD"
| "TMPDIR"
| "TEMP"
| "TMP"
| "LANG"
| "TERM"
| "XDG_CONFIG_HOME"
)
}
#[cfg(test)]
fn text_mentions_skill(text: &str, skill_name: &str) -> bool {
if skill_name.is_empty() {
return false;
}
let text_bytes = text.as_bytes();
let skill_bytes = skill_name.as_bytes();
for (index, byte) in text_bytes.iter().copied().enumerate() {
if byte != b'$' {
continue;
}
let name_start = index + 1;
let Some(rest) = text_bytes.get(name_start..) else {
continue;
};
if !rest.starts_with(skill_bytes) {
continue;
}
let after_index = name_start + skill_bytes.len();
let after = text_bytes.get(after_index).copied();
if after.is_none_or(|b| !is_mention_name_char(b)) {
return true;
}
}
false
}
fn is_mention_name_char(byte: u8) -> bool {
matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-' | b':')
}
#[cfg(test)]
#[path = "injection_tests.rs"]
mod tests;

View file

@ -1,293 +0,0 @@
use super::*;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::collections::HashSet;
fn make_skill(name: &str, path: &str) -> SkillMetadata {
SkillMetadata {
name: name.to_string(),
description: format!("{name} skill"),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf(path).abs(),
scope: codex_protocol::protocol::SkillScope::User,
}
}
fn set<'a>(items: &'a [&'a str]) -> HashSet<&'a str> {
items.iter().copied().collect()
}
fn assert_mentions(text: &str, expected_names: &[&str], expected_paths: &[&str]) {
let mentions = extract_tool_mentions(text);
assert_eq!(mentions.names, set(expected_names));
assert_eq!(mentions.paths, set(expected_paths));
}
fn linked_skill_mention(name: &str, unix_path: &str) -> String {
format!("[${name}]({})", test_path_buf(unix_path).display())
}
fn collect_mentions(
inputs: &[UserInput],
skills: &[SkillMetadata],
disabled_paths: &HashSet<AbsolutePathBuf>,
connector_slug_counts: &HashMap<String, usize>,
) -> Vec<SkillMetadata> {
collect_explicit_skill_mentions(inputs, skills, disabled_paths, connector_slug_counts)
}
#[test]
fn text_mentions_skill_requires_exact_boundary() {
assert_eq!(
true,
text_mentions_skill("use $notion-research-doc please", "notion-research-doc")
);
assert_eq!(
true,
text_mentions_skill("($notion-research-doc)", "notion-research-doc")
);
assert_eq!(
true,
text_mentions_skill("$notion-research-doc.", "notion-research-doc")
);
assert_eq!(
false,
text_mentions_skill("$notion-research-docs", "notion-research-doc")
);
assert_eq!(
false,
text_mentions_skill("$notion-research-doc_extra", "notion-research-doc")
);
}
#[test]
fn text_mentions_skill_handles_end_boundary_and_near_misses() {
assert_eq!(true, text_mentions_skill("$alpha-skill", "alpha-skill"));
assert_eq!(false, text_mentions_skill("$alpha-skillx", "alpha-skill"));
assert_eq!(
true,
text_mentions_skill("$alpha-skillx and later $alpha-skill ", "alpha-skill")
);
}
#[test]
fn text_mentions_skill_handles_many_dollars_without_looping() {
let prefix = "$".repeat(256);
let text = format!("{prefix} not-a-mention");
assert_eq!(false, text_mentions_skill(&text, "alpha-skill"));
}
#[test]
fn extract_tool_mentions_handles_plain_and_linked_mentions() {
assert_mentions(
"use $alpha and [$beta](/tmp/beta)",
&["alpha", "beta"],
&["/tmp/beta"],
);
}
#[test]
fn extract_tool_mentions_skips_common_env_vars() {
assert_mentions("use $PATH and $alpha", &["alpha"], &[]);
assert_mentions("use [$HOME](/tmp/skill)", &[], &[]);
assert_mentions("use $XDG_CONFIG_HOME and $beta", &["beta"], &[]);
}
#[test]
fn extract_tool_mentions_requires_link_syntax() {
assert_mentions("[beta](/tmp/beta)", &[], &[]);
assert_mentions("[$beta] /tmp/beta", &["beta"], &[]);
assert_mentions("[$beta]()", &["beta"], &[]);
}
#[test]
fn extract_tool_mentions_trims_linked_paths_and_allows_spacing() {
assert_mentions("use [$beta] ( /tmp/beta )", &["beta"], &["/tmp/beta"]);
}
#[test]
fn extract_tool_mentions_stops_at_non_name_chars() {
assert_mentions(
"use $alpha.skill and $beta_extra",
&["alpha", "beta_extra"],
&[],
);
}
#[test]
fn extract_tool_mentions_keeps_tool_style_namespaces() {
assert_mentions(
"use $slack:search and $alpha",
&["alpha", "slack:search"],
&[],
);
}
#[test]
fn collect_explicit_skill_mentions_text_respects_skill_order() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let beta = make_skill("beta-skill", "/tmp/beta");
let skills = vec![beta.clone(), alpha.clone()];
let inputs = vec![UserInput::Text {
text: "first $alpha-skill then $beta-skill".to_string(),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
// Text scanning should not change the previous selection ordering semantics.
assert_eq!(selected, vec![beta, alpha]);
}
#[test]
fn collect_explicit_skill_mentions_dedupes_by_path() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let skills = vec![alpha.clone()];
let mention = linked_skill_mention("alpha-skill", "/tmp/alpha");
let inputs = vec![UserInput::Text {
text: format!("use {mention} and {mention}"),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, vec![alpha]);
}
#[test]
fn collect_explicit_skill_mentions_skips_ambiguous_name() {
let alpha = make_skill("demo-skill", "/tmp/alpha");
let beta = make_skill("demo-skill", "/tmp/beta");
let skills = vec![alpha, beta];
let inputs = vec![UserInput::Text {
text: "use $demo-skill and again $demo-skill".to_string(),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, Vec::new());
}
#[test]
fn collect_explicit_skill_mentions_prefers_linked_path_over_name() {
let alpha = make_skill("demo-skill", "/tmp/alpha");
let beta = make_skill("demo-skill", "/tmp/beta");
let skills = vec![alpha, beta.clone()];
let inputs = vec![UserInput::Text {
text: format!(
"use $demo-skill and {}",
linked_skill_mention("demo-skill", "/tmp/beta")
),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, vec![beta]);
}
#[test]
fn collect_explicit_skill_mentions_skips_plain_name_when_connector_matches() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let skills = vec![alpha];
let inputs = vec![UserInput::Text {
text: "use $alpha-skill".to_string(),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::from([("alpha-skill".to_string(), 1)]);
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, Vec::new());
}
#[test]
fn collect_explicit_skill_mentions_allows_explicit_path_with_connector_conflict() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let skills = vec![alpha.clone()];
let inputs = vec![UserInput::Text {
text: format!("use {}", linked_skill_mention("alpha-skill", "/tmp/alpha")),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::from([("alpha-skill".to_string(), 1)]);
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, vec![alpha]);
}
#[test]
fn collect_explicit_skill_mentions_skips_when_linked_path_disabled() {
let alpha = make_skill("demo-skill", "/tmp/alpha");
let beta = make_skill("demo-skill", "/tmp/beta");
let skills = vec![alpha, beta];
let inputs = vec![UserInput::Text {
text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/alpha")),
text_elements: Vec::new(),
}];
let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs()]);
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts);
assert_eq!(selected, Vec::new());
}
#[test]
fn collect_explicit_skill_mentions_prefers_resource_path() {
let alpha = make_skill("demo-skill", "/tmp/alpha");
let beta = make_skill("demo-skill", "/tmp/beta");
let skills = vec![alpha, beta.clone()];
let inputs = vec![UserInput::Text {
text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/beta")),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, vec![beta]);
}
#[test]
fn collect_explicit_skill_mentions_skips_missing_path_with_no_fallback() {
let alpha = make_skill("demo-skill", "/tmp/alpha");
let beta = make_skill("demo-skill", "/tmp/beta");
let skills = vec![alpha, beta];
let inputs = vec![UserInput::Text {
text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/missing")),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, Vec::new());
}
#[test]
fn collect_explicit_skill_mentions_skips_missing_path_without_fallback() {
let alpha = make_skill("demo-skill", "/tmp/alpha");
let skills = vec![alpha];
let inputs = vec![UserInput::Text {
text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/missing")),
text_elements: Vec::new(),
}];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, Vec::new());
}

View file

@ -1,145 +0,0 @@
use std::collections::HashMap;
use std::path::Path;
use crate::SkillLoadOutcome;
use crate::SkillMetadata;
use codex_utils_absolute_path::AbsolutePathBuf;
pub(crate) fn build_implicit_skill_path_indexes(
skills: Vec<SkillMetadata>,
) -> (
HashMap<AbsolutePathBuf, SkillMetadata>,
HashMap<AbsolutePathBuf, SkillMetadata>,
) {
let mut by_scripts_dir = HashMap::new();
let mut by_skill_doc_path = HashMap::new();
for skill in skills {
let skill_doc_path = canonicalize_if_exists(&skill.path_to_skills_md);
by_skill_doc_path.insert(skill_doc_path, skill.clone());
if let Some(skill_dir) = skill.path_to_skills_md.parent() {
let scripts_dir = canonicalize_if_exists(&skill_dir.join("scripts"));
by_scripts_dir.insert(scripts_dir, skill);
}
}
(by_scripts_dir, by_skill_doc_path)
}
pub fn detect_implicit_skill_invocation_for_command(
outcome: &SkillLoadOutcome,
command: &str,
workdir: &AbsolutePathBuf,
) -> Option<SkillMetadata> {
let workdir = canonicalize_if_exists(workdir);
let tokens = tokenize_command(command);
if let Some(candidate) = detect_skill_script_run(outcome, tokens.as_slice(), &workdir) {
return Some(candidate);
}
detect_skill_doc_read(outcome, tokens.as_slice(), &workdir)
}
fn tokenize_command(command: &str) -> Vec<String> {
shlex::split(command)
.unwrap_or_else(|| command.split_whitespace().map(str::to_string).collect())
}
fn script_run_token(tokens: &[String]) -> Option<&str> {
const RUNNERS: [&str; 10] = [
"python", "python3", "bash", "zsh", "sh", "node", "deno", "ruby", "perl", "pwsh",
];
const SCRIPT_EXTENSIONS: [&str; 7] = [".py", ".sh", ".js", ".ts", ".rb", ".pl", ".ps1"];
let runner_token = tokens.first()?;
let runner = command_basename(runner_token).to_ascii_lowercase();
let runner = runner.strip_suffix(".exe").unwrap_or(&runner);
if !RUNNERS.contains(&runner) {
return None;
}
let mut script_token = None;
for token in tokens.iter().skip(1) {
if token == "--" || token.starts_with('-') {
continue;
}
script_token = Some(token.as_str());
break;
}
let script_token = script_token?;
if SCRIPT_EXTENSIONS
.iter()
.any(|extension| script_token.to_ascii_lowercase().ends_with(extension))
{
return Some(script_token);
}
None
}
fn detect_skill_script_run(
outcome: &SkillLoadOutcome,
tokens: &[String],
workdir: &AbsolutePathBuf,
) -> Option<SkillMetadata> {
let script_token = script_run_token(tokens)?;
let script_path = Path::new(script_token);
let script_path = canonicalize_if_exists(&workdir.join(script_path));
for path in script_path.ancestors() {
if let Some(candidate) = outcome.implicit_skills_by_scripts_dir.get(&path) {
return Some(candidate.clone());
}
}
None
}
fn detect_skill_doc_read(
outcome: &SkillLoadOutcome,
tokens: &[String],
workdir: &AbsolutePathBuf,
) -> Option<SkillMetadata> {
if !command_reads_file(tokens) {
return None;
}
for token in tokens.iter().skip(1) {
if token.starts_with('-') {
continue;
}
let path = Path::new(token);
let candidate_path = canonicalize_if_exists(&workdir.join(path));
if let Some(candidate) = outcome.implicit_skills_by_doc_path.get(&candidate_path) {
return Some(candidate.clone());
}
}
None
}
fn command_reads_file(tokens: &[String]) -> bool {
const READERS: [&str; 8] = ["cat", "sed", "head", "tail", "less", "more", "bat", "awk"];
let Some(program) = tokens.first() else {
return false;
};
let program = command_basename(program).to_ascii_lowercase();
READERS.contains(&program.as_str())
}
fn command_basename(command: &str) -> String {
Path::new(command)
.file_name()
.and_then(|name| name.to_str())
.unwrap_or(command)
.to_string()
}
fn canonicalize_if_exists(path: &AbsolutePathBuf) -> AbsolutePathBuf {
path.canonicalize().unwrap_or_else(|_| path.clone())
}
#[cfg(test)]
#[path = "invocation_utils_tests.rs"]
mod tests;

View file

@ -1,122 +0,0 @@
use super::SkillLoadOutcome;
use super::SkillMetadata;
use super::canonicalize_if_exists;
use super::detect_skill_doc_read;
use super::detect_skill_script_run;
use super::script_run_token;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::sync::Arc;
fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata {
SkillMetadata {
name: "test-skill".to_string(),
description: "test".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: skill_doc_path,
scope: codex_protocol::protocol::SkillScope::User,
}
}
fn test_path_display(unix_path: &str) -> String {
test_path_buf(unix_path).display().to_string()
}
#[test]
fn script_run_detection_matches_runner_plus_extension() {
let tokens = vec![
"python3".to_string(),
"-u".to_string(),
"scripts/fetch_comments.py".to_string(),
];
assert_eq!(script_run_token(&tokens).is_some(), true);
}
#[test]
fn script_run_detection_excludes_python_c() {
let tokens = vec![
"python3".to_string(),
"-c".to_string(),
"print(1)".to_string(),
];
assert_eq!(script_run_token(&tokens).is_some(), false);
}
#[test]
fn skill_doc_read_detection_matches_absolute_path() {
let skill_doc_path = test_path_buf("/tmp/skill-test/SKILL.md").abs();
let normalized_skill_doc_path = canonicalize_if_exists(&skill_doc_path);
let skill = test_skill_metadata(skill_doc_path);
let outcome = SkillLoadOutcome {
implicit_skills_by_scripts_dir: Arc::new(HashMap::new()),
implicit_skills_by_doc_path: Arc::new(HashMap::from([(normalized_skill_doc_path, skill)])),
..Default::default()
};
let tokens = vec![
"cat".to_string(),
test_path_display("/tmp/skill-test/SKILL.md"),
"|".to_string(),
"head".to_string(),
];
let found = detect_skill_doc_read(&outcome, &tokens, &test_path_buf("/tmp").abs());
assert_eq!(
found.map(|value| value.name),
Some("test-skill".to_string())
);
}
#[test]
fn skill_script_run_detection_matches_relative_path_from_skill_root() {
let skill_doc_path = test_path_buf("/tmp/skill-test/SKILL.md").abs();
let scripts_dir = canonicalize_if_exists(&test_path_buf("/tmp/skill-test/scripts").abs());
let skill = test_skill_metadata(skill_doc_path);
let outcome = SkillLoadOutcome {
implicit_skills_by_scripts_dir: Arc::new(HashMap::from([(scripts_dir, skill)])),
implicit_skills_by_doc_path: Arc::new(HashMap::new()),
..Default::default()
};
let tokens = vec![
"python3".to_string(),
"scripts/fetch_comments.py".to_string(),
];
let found = detect_skill_script_run(&outcome, &tokens, &test_path_buf("/tmp/skill-test").abs());
assert_eq!(
found.map(|value| value.name),
Some("test-skill".to_string())
);
}
#[test]
fn skill_script_run_detection_matches_absolute_path_from_any_workdir() {
let skill_doc_path = test_path_buf("/tmp/skill-test/SKILL.md").abs();
let scripts_dir = canonicalize_if_exists(&test_path_buf("/tmp/skill-test/scripts").abs());
let skill = test_skill_metadata(skill_doc_path);
let outcome = SkillLoadOutcome {
implicit_skills_by_scripts_dir: Arc::new(HashMap::from([(scripts_dir, skill)])),
implicit_skills_by_doc_path: Arc::new(HashMap::new()),
..Default::default()
};
let tokens = vec![
"python3".to_string(),
test_path_display("/tmp/skill-test/scripts/fetch_comments.py"),
];
let found = detect_skill_script_run(&outcome, &tokens, &test_path_buf("/tmp/other").abs());
assert_eq!(
found.map(|value| value.name),
Some("test-skill".to_string())
);
}

View file

@ -1,29 +0,0 @@
pub mod config_rules;
mod env_var_dependencies;
pub mod injection;
pub(crate) mod invocation_utils;
pub mod loader;
pub mod manager;
mod mention_counts;
pub mod model;
pub mod remote;
pub mod render;
pub mod system;
pub use env_var_dependencies::SkillDependencyInfo;
pub use env_var_dependencies::collect_env_var_dependencies;
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
pub use invocation_utils::detect_implicit_skill_invocation_for_command;
pub use manager::SkillsLoadInput;
pub use manager::SkillsManager;
pub use mention_counts::build_skill_name_counts;
pub use model::SkillError;
pub use model::SkillLoadOutcome;
pub use model::SkillMetadata;
pub use model::SkillPolicy;
pub use model::filter_skill_load_outcome_for_product;
pub use render::AvailableSkills;
pub use render::SkillMetadataBudget;
pub use render::SkillRenderReport;
pub use render::build_available_skills;
pub use render::default_skill_metadata_budget;

View file

@ -1,946 +0,0 @@
use crate::model::SkillDependencies;
use crate::model::SkillError;
use crate::model::SkillFileSystemsByPath;
use crate::model::SkillInterface;
use crate::model::SkillLoadOutcome;
use crate::model::SkillMetadata;
use crate::model::SkillPolicy;
use crate::model::SkillToolDependency;
use crate::system::system_cache_root_dir;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::default_project_root_markers;
use codex_config::merge_toml_values;
use codex_config::project_root_markers_from_config;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use dirs::home_dir;
use serde::Deserialize;
use std::collections::HashMap;
use std::collections::HashSet;
use std::collections::VecDeque;
use std::error::Error;
use std::fmt;
use std::io;
use std::path::Component;
use std::path::PathBuf;
use std::sync::Arc;
use toml::Value as TomlValue;
use tracing::error;
#[derive(Debug, Deserialize)]
struct SkillFrontmatter {
#[serde(default)]
name: Option<String>,
#[serde(default)]
description: Option<String>,
#[serde(default)]
metadata: SkillFrontmatterMetadata,
}
#[derive(Debug, Default, Deserialize)]
struct SkillFrontmatterMetadata {
#[serde(default, rename = "short-description")]
short_description: Option<String>,
}
#[derive(Debug, Default, Deserialize)]
struct SkillMetadataFile {
#[serde(default)]
interface: Option<Interface>,
#[serde(default)]
dependencies: Option<Dependencies>,
#[serde(default)]
policy: Option<Policy>,
}
#[derive(Default)]
struct LoadedSkillMetadata {
interface: Option<SkillInterface>,
dependencies: Option<SkillDependencies>,
policy: Option<SkillPolicy>,
}
#[derive(Debug, Default, Deserialize)]
struct Interface {
display_name: Option<String>,
short_description: Option<String>,
icon_small: Option<PathBuf>,
icon_large: Option<PathBuf>,
brand_color: Option<String>,
default_prompt: Option<String>,
}
#[derive(Debug, Default, Deserialize)]
struct Dependencies {
#[serde(default)]
tools: Vec<DependencyTool>,
}
#[derive(Debug, Deserialize)]
struct Policy {
#[serde(default)]
allow_implicit_invocation: Option<bool>,
#[serde(default)]
products: Vec<Product>,
}
#[derive(Debug, Default, Deserialize)]
struct DependencyTool {
#[serde(rename = "type")]
kind: Option<String>,
value: Option<String>,
description: Option<String>,
transport: Option<String>,
command: Option<String>,
url: Option<String>,
}
const SKILLS_FILENAME: &str = "SKILL.md";
const AGENTS_DIR_NAME: &str = ".agents";
const SKILLS_METADATA_DIR: &str = "agents";
const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
const SKILLS_DIR_NAME: &str = "skills";
const MAX_NAME_LEN: usize = 64;
const MAX_DESCRIPTION_LEN: usize = 1024;
const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEFAULT_PROMPT_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEPENDENCY_TYPE_LEN: usize = MAX_NAME_LEN;
const MAX_DEPENDENCY_TRANSPORT_LEN: usize = MAX_NAME_LEN;
const MAX_DEPENDENCY_VALUE_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEPENDENCY_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEPENDENCY_COMMAND_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEPENDENCY_URL_LEN: usize = MAX_DESCRIPTION_LEN;
// Traversal depth from the skills root.
const MAX_SCAN_DEPTH: usize = 6;
const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000;
#[derive(Debug)]
enum SkillParseError {
Read(std::io::Error),
MissingFrontmatter,
InvalidYaml(serde_yaml::Error),
MissingField(&'static str),
InvalidField { field: &'static str, reason: String },
}
impl fmt::Display for SkillParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
SkillParseError::Read(e) => write!(f, "failed to read file: {e}"),
SkillParseError::MissingFrontmatter => {
write!(f, "missing YAML frontmatter delimited by ---")
}
SkillParseError::InvalidYaml(e) => write!(f, "invalid YAML: {e}"),
SkillParseError::MissingField(field) => write!(f, "missing field `{field}`"),
SkillParseError::InvalidField { field, reason } => {
write!(f, "invalid {field}: {reason}")
}
}
}
}
impl Error for SkillParseError {}
pub struct SkillRoot {
pub path: AbsolutePathBuf,
pub scope: SkillScope,
pub file_system: Arc<dyn ExecutorFileSystem>,
}
pub async fn load_skills_from_roots<I>(roots: I) -> SkillLoadOutcome
where
I: IntoIterator<Item = SkillRoot>,
{
let mut outcome = SkillLoadOutcome::default();
let mut file_systems_by_skill_path: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>> =
HashMap::new();
for root in roots {
let fs = root.file_system;
let skills_before_root = outcome.skills.len();
discover_skills_under_root(fs.as_ref(), &root.path, root.scope, &mut outcome).await;
for skill in &outcome.skills[skills_before_root..] {
file_systems_by_skill_path
.entry(skill.path_to_skills_md.clone())
.or_insert_with(|| Arc::clone(&fs));
}
}
let mut seen: HashSet<AbsolutePathBuf> = HashSet::new();
outcome
.skills
.retain(|skill| seen.insert(skill.path_to_skills_md.clone()));
let retained_skill_paths: HashSet<AbsolutePathBuf> = outcome
.skills
.iter()
.map(|skill| skill.path_to_skills_md.clone())
.collect();
file_systems_by_skill_path.retain(|path, _| retained_skill_paths.contains(path));
outcome.file_systems_by_skill_path = SkillFileSystemsByPath::new(file_systems_by_skill_path);
fn scope_rank(scope: SkillScope) -> u8 {
// Higher-priority scopes first (matches root scan order for dedupe).
match scope {
SkillScope::Repo => 0,
SkillScope::User => 1,
SkillScope::System => 2,
SkillScope::Admin => 3,
}
}
outcome.skills.sort_by(|a, b| {
scope_rank(a.scope)
.cmp(&scope_rank(b.scope))
.then_with(|| a.name.cmp(&b.name))
.then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md))
});
outcome
}
pub(crate) async fn skill_roots(
fs: Option<Arc<dyn ExecutorFileSystem>>,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
plugin_skill_roots: Vec<AbsolutePathBuf>,
) -> Vec<SkillRoot> {
let home_dir =
home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok());
skill_roots_with_home_dir(
fs,
config_layer_stack,
cwd,
home_dir.as_ref(),
plugin_skill_roots,
)
.await
}
async fn skill_roots_with_home_dir(
fs: Option<Arc<dyn ExecutorFileSystem>>,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
home_dir: Option<&AbsolutePathBuf>,
plugin_skill_roots: Vec<AbsolutePathBuf>,
) -> Vec<SkillRoot> {
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir, fs.clone());
roots.extend(plugin_skill_roots.into_iter().map(|path| SkillRoot {
path,
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
}));
roots.extend(repo_agents_skill_roots(fs, config_layer_stack, cwd).await);
dedupe_skill_roots_by_path(&mut roots);
roots
}
fn skill_roots_from_layer_stack_inner(
config_layer_stack: &ConfigLayerStack,
home_dir: Option<&AbsolutePathBuf>,
repo_fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> Vec<SkillRoot> {
let mut roots = Vec::new();
for layer in config_layer_stack.get_layers(
ConfigLayerStackOrdering::HighestPrecedenceFirst,
/*include_disabled*/ true,
) {
let Some(config_folder) = layer.config_folder() else {
continue;
};
match &layer.name {
ConfigLayerSource::Project { .. } => {
if let Some(repo_fs) = &repo_fs {
roots.push(SkillRoot {
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::Repo,
file_system: Arc::clone(repo_fs),
});
}
}
ConfigLayerSource::User { .. } => {
// Deprecated user skills location (`$CODEX_HOME/skills`), kept for backward
// compatibility.
roots.push(SkillRoot {
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
});
// `$HOME/.agents/skills` (user-installed skills).
if let Some(home_dir) = home_dir {
roots.push(SkillRoot {
path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
});
}
// Embedded system skills are cached under `$CODEX_HOME/skills/.system` and are a
// special case (not a config layer).
roots.push(SkillRoot {
path: system_cache_root_dir(&config_folder),
scope: SkillScope::System,
file_system: Arc::clone(&LOCAL_FS),
});
}
ConfigLayerSource::System { .. } => {
// The system config layer lives under `/etc/codex/` on Unix, so treat
// `/etc/codex/skills` as admin-scoped skills.
roots.push(SkillRoot {
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::Admin,
file_system: Arc::clone(&LOCAL_FS),
});
}
ConfigLayerSource::Mdm { .. }
| ConfigLayerSource::SessionFlags
| ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. }
| ConfigLayerSource::LegacyManagedConfigTomlFromMdm => {}
}
}
roots
}
async fn repo_agents_skill_roots(
fs: Option<Arc<dyn ExecutorFileSystem>>,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
) -> Vec<SkillRoot> {
let Some(fs) = fs else {
return Vec::new();
};
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
let project_root = find_project_root(fs.as_ref(), cwd, &project_root_markers).await;
let dirs = dirs_between_project_root_and_cwd(cwd, &project_root);
let mut roots = Vec::new();
for dir in dirs {
let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
match fs.get_metadata(&agents_skills, /*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => roots.push(SkillRoot {
path: agents_skills,
scope: SkillScope::Repo,
file_system: Arc::clone(&fs),
}),
Ok(_) => {}
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(err) => {
tracing::warn!(
"failed to stat repo skills root {}: {err:#}",
agents_skills.display()
);
}
}
}
roots
}
fn project_root_markers_from_stack(config_layer_stack: &ConfigLayerStack) -> Vec<String> {
let mut merged = TomlValue::Table(toml::map::Map::new());
for layer in config_layer_stack.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ false,
) {
if matches!(layer.name, ConfigLayerSource::Project { .. }) {
continue;
}
merge_toml_values(&mut merged, &layer.config);
}
match project_root_markers_from_config(&merged) {
Ok(Some(markers)) => markers,
Ok(None) => default_project_root_markers(),
Err(err) => {
tracing::warn!("invalid project_root_markers: {err}");
default_project_root_markers()
}
}
}
async fn find_project_root(
fs: &dyn ExecutorFileSystem,
cwd: &AbsolutePathBuf,
project_root_markers: &[String],
) -> AbsolutePathBuf {
if project_root_markers.is_empty() {
return cwd.clone();
}
for ancestor in cwd.ancestors() {
for marker in project_root_markers {
let marker_path = ancestor.join(marker);
match fs.get_metadata(&marker_path, /*sandbox*/ None).await {
Ok(_) => return ancestor,
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(err) => {
tracing::warn!(
"failed to stat project root marker {}: {err:#}",
marker_path.display()
);
}
}
}
}
cwd.clone()
}
fn dirs_between_project_root_and_cwd(
cwd: &AbsolutePathBuf,
project_root: &AbsolutePathBuf,
) -> Vec<AbsolutePathBuf> {
let mut dirs = cwd
.ancestors()
.scan(false, |done, dir| {
if *done {
None
} else {
if &dir == project_root {
*done = true;
}
Some(dir)
}
})
.collect::<Vec<_>>();
dirs.reverse();
dirs
}
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
let mut seen: HashSet<AbsolutePathBuf> = HashSet::new();
roots.retain(|root| seen.insert(root.path.clone()));
}
fn canonicalize_for_skill_identity(path: &AbsolutePathBuf) -> AbsolutePathBuf {
path.canonicalize().unwrap_or_else(|_| path.clone())
}
async fn discover_skills_under_root(
fs: &dyn ExecutorFileSystem,
root: &AbsolutePathBuf,
scope: SkillScope,
outcome: &mut SkillLoadOutcome,
) {
let root = canonicalize_for_skill_identity(root);
match fs.get_metadata(&root, /*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => {}
Ok(_) => return,
Err(err) if err.kind() == io::ErrorKind::NotFound => return,
Err(err) => {
error!("failed to stat skills root {}: {err:#}", root.display());
return;
}
}
fn enqueue_dir(
queue: &mut VecDeque<(AbsolutePathBuf, usize)>,
visited_dirs: &mut HashSet<AbsolutePathBuf>,
truncated_by_dir_limit: &mut bool,
path: AbsolutePathBuf,
depth: usize,
) {
if depth > MAX_SCAN_DEPTH {
return;
}
if visited_dirs.len() >= MAX_SKILLS_DIRS_PER_ROOT {
*truncated_by_dir_limit = true;
return;
}
if visited_dirs.insert(path.clone()) {
queue.push_back((path, depth));
}
}
// Follow symlinked directories for user, admin, and repo skills. System skills are written by Codex itself.
let follow_symlinks = matches!(
scope,
SkillScope::Repo | SkillScope::User | SkillScope::Admin
);
let mut visited_dirs: HashSet<AbsolutePathBuf> = HashSet::new();
visited_dirs.insert(root.clone());
let mut queue: VecDeque<(AbsolutePathBuf, usize)> = VecDeque::from([(root.clone(), 0)]);
let mut truncated_by_dir_limit = false;
while let Some((dir, depth)) = queue.pop_front() {
let entries = match fs.read_directory(&dir, /*sandbox*/ None).await {
Ok(entries) => entries,
Err(e) => {
error!("failed to read skills dir {}: {e:#}", dir.display());
continue;
}
};
for entry in entries {
let file_name = entry.file_name;
if file_name.starts_with('.') {
continue;
}
let path = dir.join(&file_name);
let metadata = match fs.get_metadata(&path, /*sandbox*/ None).await {
Ok(metadata) => metadata,
Err(e) => {
error!("failed to stat skills path {}: {e:#}", path.display());
continue;
}
};
if metadata.is_symlink {
if !follow_symlinks {
continue;
}
match fs.read_directory(&path, /*sandbox*/ None).await {
Ok(_) => {
let resolved_dir = canonicalize_for_skill_identity(&path);
enqueue_dir(
&mut queue,
&mut visited_dirs,
&mut truncated_by_dir_limit,
resolved_dir,
depth + 1,
);
}
Err(err)
if matches!(
err.kind(),
io::ErrorKind::NotADirectory | io::ErrorKind::NotFound
) => {}
Err(err) => {
error!(
"failed to read skills symlink dir {}: {err:#}",
path.display()
);
}
}
continue;
}
if metadata.is_directory {
let resolved_dir = canonicalize_for_skill_identity(&path);
enqueue_dir(
&mut queue,
&mut visited_dirs,
&mut truncated_by_dir_limit,
resolved_dir,
depth + 1,
);
continue;
}
if metadata.is_file && file_name == SKILLS_FILENAME {
match parse_skill_file(fs, &path, scope).await {
Ok(skill) => {
outcome.skills.push(skill);
}
Err(err) => {
if scope != SkillScope::System {
outcome.errors.push(SkillError {
path: path.clone(),
message: err.to_string(),
});
}
}
}
}
}
}
if truncated_by_dir_limit {
tracing::warn!(
"skills scan truncated after {} directories (root: {})",
MAX_SKILLS_DIRS_PER_ROOT,
root.display()
);
}
}
async fn parse_skill_file(
fs: &dyn ExecutorFileSystem,
path: &AbsolutePathBuf,
scope: SkillScope,
) -> Result<SkillMetadata, SkillParseError> {
let contents = fs
.read_file_text(path, /*sandbox*/ None)
.await
.map_err(SkillParseError::Read)?;
let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?;
let parsed: SkillFrontmatter =
serde_yaml::from_str(&frontmatter).map_err(SkillParseError::InvalidYaml)?;
let base_name = parsed
.name
.as_deref()
.map(sanitize_single_line)
.filter(|value| !value.is_empty())
.unwrap_or_else(|| default_skill_name(path));
let name = base_name.to_string();
let description = parsed
.description
.as_deref()
.map(sanitize_single_line)
.unwrap_or_default();
let short_description = parsed
.metadata
.short_description
.as_deref()
.map(sanitize_single_line)
.filter(|value| !value.is_empty());
let LoadedSkillMetadata {
interface,
dependencies,
policy,
} = load_skill_metadata(fs, path).await;
validate_len(&name, MAX_NAME_LEN, "name")?;
validate_len(&description, MAX_DESCRIPTION_LEN, "description")?;
if let Some(short_description) = short_description.as_deref() {
validate_len(
short_description,
MAX_SHORT_DESCRIPTION_LEN,
"metadata.short-description",
)?;
}
let resolved_path = canonicalize_for_skill_identity(path);
Ok(SkillMetadata {
name,
description,
short_description,
interface,
dependencies,
policy,
path_to_skills_md: resolved_path,
scope,
})
}
fn default_skill_name(path: &AbsolutePathBuf) -> String {
path.parent()
.and_then(|parent| {
parent
.file_name()
.and_then(|name| name.to_str())
.map(sanitize_single_line)
})
.filter(|value| !value.is_empty())
.unwrap_or_else(|| "skill".to_string())
}
async fn load_skill_metadata(
fs: &dyn ExecutorFileSystem,
skill_path: &AbsolutePathBuf,
) -> LoadedSkillMetadata {
// Fail open: optional metadata should not block loading SKILL.md.
let Some(skill_dir) = skill_path.parent() else {
return LoadedSkillMetadata::default();
};
let metadata_path = skill_dir
.join(SKILLS_METADATA_DIR)
.join(SKILLS_METADATA_FILENAME);
match fs.get_metadata(&metadata_path, /*sandbox*/ None).await {
Ok(metadata) if metadata.is_file => {}
Ok(_) => return LoadedSkillMetadata::default(),
Err(error) if error.kind() == io::ErrorKind::NotFound => {
return LoadedSkillMetadata::default();
}
Err(error) => {
tracing::warn!(
"ignoring {path}: failed to stat {label}: {error}",
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
}
}
let contents = match fs.read_file_text(&metadata_path, /*sandbox*/ None).await {
Ok(contents) => contents,
Err(error) => {
tracing::warn!(
"ignoring {path}: failed to read {label}: {error}",
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
}
};
let parsed: SkillMetadataFile = {
let _guard = AbsolutePathBufGuard::new(skill_dir.as_path());
match serde_yaml::from_str(&contents) {
Ok(parsed) => parsed,
Err(error) => {
tracing::warn!(
"ignoring {path}: invalid {label}: {error}",
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
}
}
};
let SkillMetadataFile {
interface,
dependencies,
policy,
} = parsed;
LoadedSkillMetadata {
interface: resolve_interface(interface, &skill_dir),
dependencies: resolve_dependencies(dependencies),
policy: resolve_policy(policy),
}
}
fn resolve_interface(
interface: Option<Interface>,
skill_dir: &AbsolutePathBuf,
) -> Option<SkillInterface> {
let interface = interface?;
let interface = SkillInterface {
display_name: resolve_str(
interface.display_name,
MAX_NAME_LEN,
"interface.display_name",
),
short_description: resolve_str(
interface.short_description,
MAX_SHORT_DESCRIPTION_LEN,
"interface.short_description",
),
icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small),
icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large),
brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"),
default_prompt: resolve_str(
interface.default_prompt,
MAX_DEFAULT_PROMPT_LEN,
"interface.default_prompt",
),
};
let has_fields = interface.display_name.is_some()
|| interface.short_description.is_some()
|| interface.icon_small.is_some()
|| interface.icon_large.is_some()
|| interface.brand_color.is_some()
|| interface.default_prompt.is_some();
if has_fields { Some(interface) } else { None }
}
fn resolve_dependencies(dependencies: Option<Dependencies>) -> Option<SkillDependencies> {
let dependencies = dependencies?;
let tools: Vec<SkillToolDependency> = dependencies
.tools
.into_iter()
.filter_map(resolve_dependency_tool)
.collect();
if tools.is_empty() {
None
} else {
Some(SkillDependencies { tools })
}
}
fn resolve_policy(policy: Option<Policy>) -> Option<SkillPolicy> {
policy.map(|policy| SkillPolicy {
allow_implicit_invocation: policy.allow_implicit_invocation,
products: policy.products,
})
}
fn resolve_dependency_tool(tool: DependencyTool) -> Option<SkillToolDependency> {
let r#type = resolve_required_str(
tool.kind,
MAX_DEPENDENCY_TYPE_LEN,
"dependencies.tools.type",
)?;
let value = resolve_required_str(
tool.value,
MAX_DEPENDENCY_VALUE_LEN,
"dependencies.tools.value",
)?;
let description = resolve_str(
tool.description,
MAX_DEPENDENCY_DESCRIPTION_LEN,
"dependencies.tools.description",
);
let transport = resolve_str(
tool.transport,
MAX_DEPENDENCY_TRANSPORT_LEN,
"dependencies.tools.transport",
);
let command = resolve_str(
tool.command,
MAX_DEPENDENCY_COMMAND_LEN,
"dependencies.tools.command",
);
let url = resolve_str(tool.url, MAX_DEPENDENCY_URL_LEN, "dependencies.tools.url");
Some(SkillToolDependency {
r#type,
value,
description,
transport,
command,
url,
})
}
fn resolve_asset_path(
skill_dir: &AbsolutePathBuf,
field: &'static str,
path: Option<PathBuf>,
) -> Option<AbsolutePathBuf> {
// Icons must be relative paths under the skill's assets/ directory; otherwise return None.
let path = path?;
if path.as_os_str().is_empty() {
return None;
}
let assets_dir = skill_dir.join("assets");
if path.is_absolute() {
tracing::warn!(
"ignoring {field}: icon must be a relative assets path (not {})",
assets_dir.display()
);
return None;
}
let mut normalized = PathBuf::new();
for component in path.components() {
match component {
Component::CurDir => {}
Component::Normal(component) => normalized.push(component),
Component::ParentDir => {
tracing::warn!("ignoring {field}: icon path must not contain '..'");
return None;
}
_ => {
tracing::warn!("ignoring {field}: icon path must be under assets/");
return None;
}
}
}
let mut components = normalized.components();
match components.next() {
Some(Component::Normal(component)) if component == "assets" => {}
_ => {
tracing::warn!("ignoring {field}: icon path must be under assets/");
return None;
}
}
Some(skill_dir.join(normalized))
}
fn sanitize_single_line(raw: &str) -> String {
raw.split_whitespace().collect::<Vec<_>>().join(" ")
}
fn validate_len(
value: &str,
max_len: usize,
field_name: &'static str,
) -> Result<(), SkillParseError> {
if value.is_empty() {
return Err(SkillParseError::MissingField(field_name));
}
if value.chars().count() > max_len {
return Err(SkillParseError::InvalidField {
field: field_name,
reason: format!("exceeds maximum length of {max_len} characters"),
});
}
Ok(())
}
fn resolve_str(value: Option<String>, max_len: usize, field: &'static str) -> Option<String> {
let value = value?;
let value = sanitize_single_line(&value);
if value.is_empty() {
tracing::warn!("ignoring {field}: value is empty");
return None;
}
if value.chars().count() > max_len {
tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters");
return None;
}
Some(value)
}
fn resolve_required_str(
value: Option<String>,
max_len: usize,
field: &'static str,
) -> Option<String> {
let Some(value) = value else {
tracing::warn!("ignoring {field}: value is missing");
return None;
};
resolve_str(Some(value), max_len, field)
}
fn resolve_color_str(value: Option<String>, field: &'static str) -> Option<String> {
let value = value?;
let value = value.trim();
if value.is_empty() {
tracing::warn!("ignoring {field}: value is empty");
return None;
}
let mut chars = value.chars();
if value.len() == 7 && chars.next() == Some('#') && chars.all(|c| c.is_ascii_hexdigit()) {
Some(value.to_string())
} else {
tracing::warn!("ignoring {field}: expected #RRGGBB, got {value}");
None
}
}
fn extract_frontmatter(contents: &str) -> Option<String> {
let mut lines = contents.lines();
if !matches!(lines.next(), Some(line) if line.trim() == "---") {
return None;
}
let mut frontmatter_lines: Vec<&str> = Vec::new();
let mut found_closing = false;
for line in lines.by_ref() {
if line.trim() == "---" {
found_closing = true;
break;
}
frontmatter_lines.push(line);
}
if frontmatter_lines.is_empty() || !found_closing {
return None;
}
Some(frontmatter_lines.join("\n"))
}
#[cfg(test)]
pub(crate) async fn skill_roots_from_layer_stack(
fs: Arc<dyn ExecutorFileSystem>,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
home_dir: Option<&AbsolutePathBuf>,
) -> Vec<SkillRoot> {
skill_roots_with_home_dir(Some(fs), config_layer_stack, cwd, home_dir, Vec::new()).await
}
#[cfg(test)]
#[path = "loader_tests.rs"]
mod tests;

File diff suppressed because it is too large Load diff

View file

@ -1,310 +0,0 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use std::sync::RwLock;
use codex_config::ConfigLayerStack;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use tracing::info;
use tracing::warn;
use crate::SkillLoadOutcome;
use crate::build_implicit_skill_path_indexes;
use crate::config_rules::SkillConfigRules;
use crate::config_rules::resolve_disabled_skill_paths;
use crate::config_rules::skill_config_rules_from_stack;
use crate::loader::SkillRoot;
use crate::loader::load_skills_from_roots;
use crate::loader::skill_roots;
use crate::system::uninstall_system_skills;
use codex_config::SkillsConfig;
#[derive(Debug, Clone)]
pub struct SkillsLoadInput {
pub cwd: AbsolutePathBuf,
pub effective_skill_roots: Vec<AbsolutePathBuf>,
pub config_layer_stack: ConfigLayerStack,
pub bundled_skills_enabled: bool,
}
impl SkillsLoadInput {
pub fn new(
cwd: AbsolutePathBuf,
effective_skill_roots: Vec<AbsolutePathBuf>,
config_layer_stack: ConfigLayerStack,
bundled_skills_enabled: bool,
) -> Self {
Self {
cwd,
effective_skill_roots,
config_layer_stack,
bundled_skills_enabled,
}
}
}
pub struct SkillsManager {
codex_home: AbsolutePathBuf,
restriction_product: Option<Product>,
cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, SkillLoadOutcome>>,
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
}
impl SkillsManager {
pub fn new(codex_home: AbsolutePathBuf, bundled_skills_enabled: bool) -> Self {
Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex))
}
pub fn new_with_restriction_product(
codex_home: AbsolutePathBuf,
bundled_skills_enabled: bool,
restriction_product: Option<Product>,
) -> Self {
let manager = Self {
codex_home,
restriction_product,
cache_by_cwd: RwLock::new(HashMap::new()),
cache_by_config: RwLock::new(HashMap::new()),
};
if !bundled_skills_enabled {
// The loader caches bundled skills under `skills/.system`. Clearing that directory is
// best-effort cleanup; root selection still enforces the config even if removal fails.
uninstall_system_skills(&manager.codex_home);
}
manager
}
/// Load skills for an already-constructed [`Config`], avoiding any additional config-layer
/// loading.
///
/// This path uses a cache keyed by the effective skill-relevant config state rather than just
/// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen
/// to share a directory.
pub async fn skills_for_config(
&self,
input: &SkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome {
let roots = self.skill_roots_for_config(input, fs).await;
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let cache_key = config_skills_cache_key(&roots, &skill_config_rules);
if let Some(outcome) = self.cached_outcome_for_config(&cache_key) {
return outcome;
}
let outcome = self.build_skill_outcome(roots, &skill_config_rules).await;
let mut cache = self
.cache_by_config
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
cache.insert(cache_key, outcome.clone());
outcome
}
pub async fn skill_roots_for_config(
&self,
input: &SkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> Vec<SkillRoot> {
let mut roots = skill_roots(
fs,
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
)
.await;
if !input.bundled_skills_enabled {
roots.retain(|root| root.scope != SkillScope::System);
}
roots
}
pub async fn skills_for_cwd(
&self,
input: &SkillsLoadInput,
force_reload: bool,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome {
self.skills_for_cwd_with_extra_user_roots(input, force_reload, &[], fs)
.await
}
pub async fn skills_for_cwd_with_extra_user_roots(
&self,
input: &SkillsLoadInput,
force_reload: bool,
extra_user_roots: &[AbsolutePathBuf],
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome {
let use_cwd_cache = fs.is_some();
if use_cwd_cache
&& !force_reload
&& let Some(outcome) = self.cached_outcome_for_cwd(&input.cwd)
{
return outcome;
}
let mut roots = skill_roots(
fs.clone(),
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
)
.await;
if !bundled_skills_enabled_from_stack(&input.config_layer_stack) {
roots.retain(|root| root.scope != SkillScope::System);
}
if let Some(fs) = fs {
roots.extend(
normalize_extra_user_roots(extra_user_roots)
.into_iter()
.map(|path| SkillRoot {
path,
scope: SkillScope::User,
file_system: Arc::clone(&fs),
}),
);
}
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let outcome = self.build_skill_outcome(roots, &skill_config_rules).await;
if use_cwd_cache {
let mut cache = self
.cache_by_cwd
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
cache.insert(input.cwd.clone(), outcome.clone());
}
outcome
}
async fn build_skill_outcome(
&self,
roots: Vec<SkillRoot>,
skill_config_rules: &SkillConfigRules,
) -> SkillLoadOutcome {
let outcome = crate::filter_skill_load_outcome_for_product(
load_skills_from_roots(roots).await,
self.restriction_product,
);
let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules);
finalize_skill_outcome(outcome, disabled_paths)
}
pub fn clear_cache(&self) {
let cleared_cwd = {
let mut cache = self
.cache_by_cwd
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let cleared = cache.len();
cache.clear();
cleared
};
let cleared_config = {
let mut cache = self
.cache_by_config
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let cleared = cache.len();
cache.clear();
cleared
};
let cleared = cleared_cwd + cleared_config;
info!("skills cache cleared ({cleared} entries)");
}
fn cached_outcome_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option<SkillLoadOutcome> {
match self.cache_by_cwd.read() {
Ok(cache) => cache.get(cwd).cloned(),
Err(err) => err.into_inner().get(cwd).cloned(),
}
}
fn cached_outcome_for_config(
&self,
cache_key: &ConfigSkillsCacheKey,
) -> Option<SkillLoadOutcome> {
match self.cache_by_config.read() {
Ok(cache) => cache.get(cache_key).cloned(),
Err(err) => err.into_inner().get(cache_key).cloned(),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillsCacheKey {
roots: Vec<(AbsolutePathBuf, u8)>,
skill_config_rules: SkillConfigRules,
}
pub fn bundled_skills_enabled_from_stack(
config_layer_stack: &codex_config::ConfigLayerStack,
) -> bool {
let effective_config = config_layer_stack.effective_config();
let Some(skills_value) = effective_config
.as_table()
.and_then(|table| table.get("skills"))
else {
return true;
};
let skills: SkillsConfig = match skills_value.clone().try_into() {
Ok(skills) => skills,
Err(err) => {
warn!("invalid skills config: {err}");
return true;
}
};
skills.bundled.unwrap_or_default().enabled
}
fn config_skills_cache_key(
roots: &[SkillRoot],
skill_config_rules: &SkillConfigRules,
) -> ConfigSkillsCacheKey {
ConfigSkillsCacheKey {
roots: roots
.iter()
.map(|root| {
let scope_rank = match root.scope {
SkillScope::Repo => 0,
SkillScope::User => 1,
SkillScope::System => 2,
SkillScope::Admin => 3,
};
(root.path.clone(), scope_rank)
})
.collect(),
skill_config_rules: skill_config_rules.clone(),
}
}
fn finalize_skill_outcome(
mut outcome: SkillLoadOutcome,
disabled_paths: HashSet<AbsolutePathBuf>,
) -> SkillLoadOutcome {
outcome.disabled_paths = disabled_paths;
let (by_scripts_dir, by_doc_path) =
build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation());
outcome.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir);
outcome.implicit_skills_by_doc_path = Arc::new(by_doc_path);
outcome
}
fn normalize_extra_user_roots(extra_user_roots: &[AbsolutePathBuf]) -> Vec<AbsolutePathBuf> {
let mut normalized: Vec<AbsolutePathBuf> = extra_user_roots
.iter()
.map(|root| root.canonicalize().unwrap_or_else(|_| root.clone()))
.collect();
normalized.sort_unstable();
normalized.dedup();
normalized
}
#[cfg(test)]
#[path = "manager_tests.rs"]
mod tests;

View file

@ -1,750 +0,0 @@
use super::*;
use crate::SkillMetadata;
use crate::config_rules::resolve_disabled_skill_paths;
use crate::config_rules::skill_config_rules_from_stack;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::CONFIG_TOML_FILE;
use codex_config::ConfigLayerEntry;
use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirementsToml;
use codex_exec_server::LOCAL_FS;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::PathExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::fs;
use std::path::PathBuf;
use std::sync::Arc;
use tempfile::TempDir;
fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) {
let skill_dir = codex_home.path().join("skills").join(dir);
fs::create_dir_all(&skill_dir).unwrap();
let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
fs::write(skill_dir.join("SKILL.md"), content).unwrap();
}
fn write_extra_root_skill(
root: &TempDir,
dir: &str,
name: &str,
description: &str,
) -> PathBuf {
let skill_dir = root.path().join("skills").join(dir);
fs::create_dir_all(&skill_dir).unwrap();
let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
let skill_path = skill_dir.join("SKILL.md");
fs::write(&skill_path, content).unwrap();
skill_path
}
fn test_skill(name: &str, path: PathBuf) -> SkillMetadata {
SkillMetadata {
name: name.to_string(),
description: "test".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: path
.abs()
.canonicalize()
.expect("skill path should canonicalize"),
scope: SkillScope::User,
}
}
fn write_demo_skill(tempdir: &TempDir) -> PathBuf {
let skill_path = tempdir.path().join("skills").join("demo").join("SKILL.md");
fs::create_dir_all(skill_path.parent().expect("skill path should have parent"))
.expect("create skill dir");
fs::write(
&skill_path,
"---\nname: demo-skill\ndescription: demo description\n---\n\n# Body\n",
)
.expect("write skill");
skill_path
}
fn user_config_layer(codex_home: &TempDir, config_toml: &str) -> ConfigLayerEntry {
let config_path = AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE))
.expect("user config path should be absolute");
ConfigLayerEntry::new(
ConfigLayerSource::User { file: config_path },
toml::from_str(config_toml).expect("user layer toml"),
)
}
fn config_stack(codex_home: &TempDir, user_config_toml: &str) -> ConfigLayerStack {
ConfigLayerStack::new(
vec![user_config_layer(codex_home, user_config_toml)],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack")
}
fn config_stack_with_session_flags(
codex_home: &TempDir,
user_config_toml: &str,
session_flags_toml: &str,
) -> ConfigLayerStack {
ConfigLayerStack::new(
vec![
user_config_layer(codex_home, user_config_toml),
ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(session_flags_toml).expect("session layer toml"),
),
],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack")
}
fn path_toggle_config(path: &std::path::Path, enabled: bool) -> String {
format!(
r#"[[skills.config]]
path = "{}"
enabled = {enabled}
"#,
path.display()
)
}
fn name_toggle_config(name: &str, enabled: bool) -> String {
format!(
r#"[[skills.config]]
name = "{name}"
enabled = {enabled}
"#
)
}
async fn skills_for_config_with_stack(
skills_manager: &SkillsManager,
cwd: &TempDir,
config_layer_stack: &ConfigLayerStack,
effective_skill_roots: &[AbsolutePathBuf],
) -> SkillLoadOutcome {
let skills_input = SkillsLoadInput::new(
cwd.path().abs(),
effective_skill_roots.to_vec(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(config_layer_stack),
);
skills_manager
.skills_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS)))
.await
}
#[test]
fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
let codex_home = tempfile::tempdir().expect("tempdir");
let stale_system_skill_dir = codex_home.path().join("skills/.system/stale-skill");
fs::create_dir_all(&stale_system_skill_dir).expect("create stale system skill dir");
fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n")
.expect("write stale system skill");
let _skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ false,
);
assert!(
!codex_home.path().join("skills/.system").exists(),
"expected disabling system skills to remove stale cached bundled skills"
);
}
#[tokio::test]
async fn skills_for_config_reuses_cache_for_same_effective_config() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
write_user_skill(&codex_home, "a", "skill-a", "from a");
let outcome1 =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
assert!(
outcome1.skills.iter().any(|s| s.name == "skill-a"),
"expected skill-a to be discovered"
);
// Write a new skill after the first call; the second call should reuse the config-aware cache
// entry because the effective skill config is unchanged.
write_user_skill(&codex_home, "b", "skill-b", "from b");
let outcome2 =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
assert_eq!(outcome2.errors, outcome1.errors);
assert_eq!(outcome2.skills, outcome1.skills);
}
#[tokio::test]
async fn skills_for_config_disables_extra_root_skills_by_name() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir");
let skill_path = write_extra_root_skill(
&extra_root,
"sample-search",
"sample-search",
"search sample data",
);
let config_layer_stack = config_stack(
&codex_home,
&name_toggle_config("sample-search", /*enabled*/ false),
);
let extra_skill_root = skill_path
.parent()
.and_then(std::path::Path::parent)
.expect("skill should live under a skills root")
.abs();
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let outcome = skills_for_config_with_stack(
&skills_manager,
&cwd,
&config_layer_stack,
&[extra_skill_root],
)
.await;
let skill = outcome
.skills
.iter()
.find(|skill| skill.name == "sample-search")
.expect("extra root skill should load");
let skill_path = dunce::canonicalize(skill_path)
.expect("skill path should canonicalize")
.abs();
assert_eq!(skill.path_to_skills_md, skill_path);
assert!(outcome.disabled_paths.contains(&skill.path_to_skills_md));
assert!(
!outcome
.allowed_skills_for_implicit_invocation()
.iter()
.any(|allowed_skill| allowed_skill.path_to_skills_md == skill.path_to_skills_md)
);
}
#[tokio::test]
async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
write_user_skill(&extra_root, "x", "extra-skill", "from extra root");
let extra_root_path = extra_root.path().abs();
let base_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let outcome_with_extra = skills_manager
.skills_for_cwd_with_extra_user_roots(
&base_input,
/*force_reload*/ true,
std::slice::from_ref(&extra_root_path),
Some(Arc::clone(&LOCAL_FS)),
)
.await;
assert!(
outcome_with_extra
.skills
.iter()
.any(|skill| skill.name == "extra-skill")
);
// The cwd-only API returns the current cached entry for this cwd, even when that entry
// was produced with extra roots.
let base_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let outcome_without_extra = skills_manager
.skills_for_cwd(
&base_input,
/*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)),
)
.await;
assert_eq!(outcome_without_extra.skills, outcome_with_extra.skills);
assert_eq!(outcome_without_extra.errors, outcome_with_extra.errors);
}
#[tokio::test]
async fn skills_for_cwd_loads_repo_user_and_extra_roots_with_local_fs() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir");
let repo_dot_codex = cwd.path().join(".codex");
fs::create_dir_all(&repo_dot_codex).expect("create repo config dir");
write_user_skill(&codex_home, "user", "user-skill", "from local user root");
write_user_skill(&extra_root, "extra", "extra-skill", "from extra root");
let repo_skill_dir = repo_dot_codex.join("skills/repo");
fs::create_dir_all(&repo_skill_dir).expect("create repo skill dir");
fs::write(
repo_skill_dir.join("SKILL.md"),
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
)
.expect("write repo skill");
let config_layer_stack = ConfigLayerStack::new(
vec![
user_config_layer(&codex_home, ""),
ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: repo_dot_codex.abs(),
},
toml::Value::Table(toml::map::Map::new()),
),
],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let skills_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let outcome = skills_manager
.skills_for_cwd_with_extra_user_roots(
&skills_input,
/*force_reload*/ true,
&[extra_root.path().abs()],
Some(Arc::clone(&LOCAL_FS)),
)
.await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
let loaded_names = outcome
.skills
.iter()
.map(|skill| skill.name.as_str())
.collect::<HashSet<_>>();
assert!(loaded_names.contains("user-skill"));
assert!(loaded_names.contains("repo-skill"));
assert!(loaded_names.contains("extra-skill"));
}
#[tokio::test]
async fn skills_for_cwd_without_fs_skips_repo_and_extra_roots() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir");
let repo_dot_codex = cwd.path().join(".codex");
fs::create_dir_all(&repo_dot_codex).expect("create repo config dir");
write_user_skill(&codex_home, "user", "user-skill", "from local user root");
write_user_skill(&extra_root, "extra", "extra-skill", "from extra root");
let repo_skill_dir = repo_dot_codex.join("skills/repo");
fs::create_dir_all(&repo_skill_dir).expect("create repo skill dir");
fs::write(
repo_skill_dir.join("SKILL.md"),
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
)
.expect("write repo skill");
let config_layer_stack = ConfigLayerStack::new(
vec![
user_config_layer(&codex_home, ""),
ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: repo_dot_codex.abs(),
},
toml::Value::Table(toml::map::Map::new()),
),
],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let skills_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let outcome = skills_manager
.skills_for_cwd_with_extra_user_roots(
&skills_input,
/*force_reload*/ true,
&[extra_root.path().abs()],
/*fs*/ None,
)
.await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
let loaded_names = outcome
.skills
.iter()
.map(|skill| skill.name.as_str())
.collect::<HashSet<_>>();
assert!(loaded_names.contains("user-skill"));
assert!(!loaded_names.contains("repo-skill"));
assert!(!loaded_names.contains("extra-skill"));
}
#[tokio::test]
async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let bundled_skill_dir = codex_home.path().join("skills/.system/bundled-skill");
fs::create_dir_all(&bundled_skill_dir).expect("create bundled skill dir");
fs::write(
bundled_skill_dir.join("SKILL.md"),
"---\nname: bundled-skill\ndescription: from bundled root\n---\n\n# Body\n",
)
.expect("write bundled skill");
let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n");
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ false,
);
// Recreate the cached bundled skill after startup cleanup so this assertion exercises
// root selection rather than relying on directory removal succeeding.
fs::create_dir_all(&bundled_skill_dir).expect("recreate bundled skill dir");
fs::write(
bundled_skill_dir.join("SKILL.md"),
"---\nname: bundled-skill\ndescription: from bundled root\n---\n\n# Body\n",
)
.expect("rewrite bundled skill");
let outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
assert!(
outcome
.skills
.iter()
.all(|skill| skill.name != "bundled-skill")
);
assert!(
outcome
.skills
.iter()
.all(|skill| skill.scope != SkillScope::System)
);
}
#[tokio::test]
async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root_a = tempfile::tempdir().expect("tempdir");
let extra_root_b = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
write_user_skill(&extra_root_a, "x", "extra-skill-a", "from extra root a");
write_user_skill(&extra_root_b, "x", "extra-skill-b", "from extra root b");
let extra_root_a_path = extra_root_a.path().abs();
let base_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let outcome_a = skills_manager
.skills_for_cwd_with_extra_user_roots(
&base_input,
/*force_reload*/ true,
std::slice::from_ref(&extra_root_a_path),
Some(Arc::clone(&LOCAL_FS)),
)
.await;
assert!(
outcome_a
.skills
.iter()
.any(|skill| skill.name == "extra-skill-a")
);
assert!(
outcome_a
.skills
.iter()
.all(|skill| skill.name != "extra-skill-b")
);
let extra_root_b_path = extra_root_b.path().abs();
let outcome_b = skills_manager
.skills_for_cwd_with_extra_user_roots(
&base_input,
/*force_reload*/ false,
std::slice::from_ref(&extra_root_b_path),
Some(Arc::clone(&LOCAL_FS)),
)
.await;
assert!(
outcome_b
.skills
.iter()
.any(|skill| skill.name == "extra-skill-a")
);
assert!(
outcome_b
.skills
.iter()
.all(|skill| skill.name != "extra-skill-b")
);
let outcome_reloaded = skills_manager
.skills_for_cwd_with_extra_user_roots(
&base_input,
/*force_reload*/ true,
std::slice::from_ref(&extra_root_b_path),
Some(Arc::clone(&LOCAL_FS)),
)
.await;
assert!(
outcome_reloaded
.skills
.iter()
.any(|skill| skill.name == "extra-skill-b")
);
assert!(
outcome_reloaded
.skills
.iter()
.all(|skill| skill.name != "extra-skill-a")
);
}
#[test]
fn normalize_extra_user_roots_is_stable_for_equivalent_inputs() {
let a = test_path_buf("/tmp/a").abs();
let b = test_path_buf("/tmp/b").abs();
let first = normalize_extra_user_roots(&[a.clone(), b.clone(), a.clone()]);
let second = normalize_extra_user_roots(&[b, a]);
assert_eq!(first, second);
}
#[cfg_attr(windows, ignore)]
#[test]
fn disabled_paths_for_skills_allows_session_flags_to_override_user_layer() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_path = write_demo_skill(&tempdir);
let skill = test_skill("demo-skill", skill_path.clone());
let user_file = AbsolutePathBuf::try_from(tempdir.path().join("config.toml"))
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&path_toggle_config(&skill_path, /*enabled*/ false))
.expect("user layer toml"),
);
let session_layer = ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(&path_toggle_config(&skill_path, /*enabled*/ true))
.expect("session layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer, session_layer],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let skill_config_rules = skill_config_rules_from_stack(&stack);
assert_eq!(
resolve_disabled_skill_paths(&[skill], &skill_config_rules),
HashSet::new()
);
}
#[cfg_attr(windows, ignore)]
#[test]
fn disabled_paths_for_skills_allows_session_flags_to_disable_user_enabled_skill() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_path = write_demo_skill(&tempdir);
let skill = test_skill("demo-skill", skill_path.clone());
let user_file = AbsolutePathBuf::try_from(tempdir.path().join("config.toml"))
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&path_toggle_config(&skill_path, /*enabled*/ true))
.expect("user layer toml"),
);
let session_layer = ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(&path_toggle_config(&skill_path, /*enabled*/ false))
.expect("session layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer, session_layer],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let skill_config_rules = skill_config_rules_from_stack(&stack);
assert_eq!(
resolve_disabled_skill_paths(&[skill], &skill_config_rules),
HashSet::from([skill_path
.abs()
.canonicalize()
.expect("skill path should canonicalize")])
);
}
#[cfg_attr(windows, ignore)]
#[test]
fn disabled_paths_for_skills_disables_matching_name_selectors() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_path = write_demo_skill(&tempdir);
let skill = test_skill("github:yeet", skill_path.clone());
let user_file = AbsolutePathBuf::try_from(tempdir.path().join("config.toml"))
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&name_toggle_config("github:yeet", /*enabled*/ false))
.expect("user layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let skill_config_rules = skill_config_rules_from_stack(&stack);
assert_eq!(
resolve_disabled_skill_paths(&[skill], &skill_config_rules),
HashSet::from([skill_path
.abs()
.canonicalize()
.expect("skill path should canonicalize")])
);
}
#[cfg_attr(windows, ignore)]
#[test]
fn disabled_paths_for_skills_allows_name_selector_to_override_path_selector() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_path = write_demo_skill(&tempdir);
let skill = test_skill("github:yeet", skill_path.clone());
let user_file = AbsolutePathBuf::try_from(tempdir.path().join("config.toml"))
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&path_toggle_config(&skill_path, /*enabled*/ false))
.expect("user layer toml"),
);
let session_layer = ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(&name_toggle_config("github:yeet", /*enabled*/ true))
.expect("session layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer, session_layer],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let skill_config_rules = skill_config_rules_from_stack(&stack);
assert_eq!(
resolve_disabled_skill_paths(&[skill], &skill_config_rules),
HashSet::new()
);
}
#[cfg_attr(windows, ignore)]
#[tokio::test]
async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let skill_dir = codex_home.path().join("skills").join("demo");
fs::create_dir_all(&skill_dir).expect("create skill dir");
let skill_path = skill_dir.join("SKILL.md");
fs::write(
&skill_path,
"---\nname: demo-skill\ndescription: demo description\n---\n\n# Body\n",
)
.expect("write skill");
let disabled_skill_config = path_toggle_config(&skill_path, /*enabled*/ false);
let enabled_skill_config = path_toggle_config(&skill_path, /*enabled*/ true);
let parent_stack = config_stack(&codex_home, &disabled_skill_config);
let child_stack =
config_stack_with_session_flags(&codex_home, &disabled_skill_config, &enabled_skill_config);
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let parent_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
parent_stack.clone(),
bundled_skills_enabled_from_stack(&parent_stack),
);
let parent_outcome = skills_manager
.skills_for_cwd(
&parent_input,
/*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)),
)
.await;
let parent_skill = parent_outcome
.skills
.iter()
.find(|skill| skill.name == "demo-skill")
.expect("demo skill should be discovered");
assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false);
let child_outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &child_stack, &[]).await;
let child_skill = child_outcome
.skills
.iter()
.find(|skill| skill.name == "demo-skill")
.expect("demo skill should be discovered");
assert_eq!(child_outcome.is_skill_enabled(child_skill), true);
}

View file

@ -1,24 +0,0 @@
use std::collections::HashMap;
use std::collections::HashSet;
use super::SkillMetadata;
use codex_utils_absolute_path::AbsolutePathBuf;
/// Counts how often each skill name appears (exact and ASCII-lowercase), excluding disabled paths.
pub fn build_skill_name_counts(
skills: &[SkillMetadata],
disabled_paths: &HashSet<AbsolutePathBuf>,
) -> (HashMap<String, usize>, HashMap<String, usize>) {
let mut exact_counts: HashMap<String, usize> = HashMap::new();
let mut lower_counts: HashMap<String, usize> = HashMap::new();
for skill in skills {
if disabled_paths.contains(&skill.path_to_skills_md) {
continue;
}
*exact_counts.entry(skill.name.clone()).or_insert(0) += 1;
*lower_counts
.entry(skill.name.to_ascii_lowercase())
.or_insert(0) += 1;
}
(exact_counts, lower_counts)
}

View file

@ -1,196 +0,0 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::sync::Arc;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
#[derive(Debug, Clone, PartialEq)]
pub struct SkillMetadata {
pub name: String,
pub description: String,
pub short_description: Option<String>,
pub interface: Option<SkillInterface>,
pub dependencies: Option<SkillDependencies>,
pub policy: Option<SkillPolicy>,
/// Path to the SKILLS.md file that declares this skill.
pub path_to_skills_md: AbsolutePathBuf,
pub scope: SkillScope,
}
impl SkillMetadata {
fn allow_implicit_invocation(&self) -> bool {
self.policy
.as_ref()
.and_then(|policy| policy.allow_implicit_invocation)
.unwrap_or(true)
}
pub fn matches_product_restriction_for_product(
&self,
restriction_product: Option<Product>,
) -> bool {
match &self.policy {
Some(policy) => {
policy.products.is_empty()
|| restriction_product.is_some_and(|product| {
product.matches_product_restriction(&policy.products)
})
}
None => true,
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub struct SkillPolicy {
pub allow_implicit_invocation: Option<bool>,
// TODO: Enforce product gating in Codex skill selection/injection instead of only parsing and
// storing this metadata.
pub products: Vec<Product>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillInterface {
pub display_name: Option<String>,
pub short_description: Option<String>,
pub icon_small: Option<AbsolutePathBuf>,
pub icon_large: Option<AbsolutePathBuf>,
pub brand_color: Option<String>,
pub default_prompt: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillDependencies {
pub tools: Vec<SkillToolDependency>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillToolDependency {
pub r#type: String,
pub value: String,
pub description: Option<String>,
pub transport: Option<String>,
pub command: Option<String>,
pub url: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillError {
pub path: AbsolutePathBuf,
pub message: String,
}
#[derive(Debug, Clone, Default)]
pub struct SkillLoadOutcome {
pub skills: Vec<SkillMetadata>,
pub errors: Vec<SkillError>,
pub disabled_paths: HashSet<AbsolutePathBuf>,
pub(crate) file_systems_by_skill_path: SkillFileSystemsByPath,
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
}
impl SkillLoadOutcome {
pub fn is_skill_enabled(&self, skill: &SkillMetadata) -> bool {
!self.disabled_paths.contains(&skill.path_to_skills_md)
}
pub fn is_skill_allowed_for_implicit_invocation(&self, skill: &SkillMetadata) -> bool {
self.is_skill_enabled(skill) && skill.allow_implicit_invocation()
}
pub fn allowed_skills_for_implicit_invocation(&self) -> Vec<SkillMetadata> {
self.skills
.iter()
.filter(|skill| self.is_skill_allowed_for_implicit_invocation(skill))
.cloned()
.collect()
}
pub fn skills_with_enabled(&self) -> impl Iterator<Item = (&SkillMetadata, bool)> {
self.skills
.iter()
.map(|skill| (skill, self.is_skill_enabled(skill)))
}
pub(crate) fn file_system_for_skill(
&self,
skill: &SkillMetadata,
) -> Option<Arc<dyn ExecutorFileSystem>> {
self.file_systems_by_skill_path
.get(&skill.path_to_skills_md)
}
}
#[derive(Clone, Default)]
pub(crate) struct SkillFileSystemsByPath {
values: Arc<HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>>,
}
impl SkillFileSystemsByPath {
pub(crate) fn new(values: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>) -> Self {
Self {
values: Arc::new(values),
}
}
fn get(&self, path: &AbsolutePathBuf) -> Option<Arc<dyn ExecutorFileSystem>> {
self.values.get(path).map(Arc::clone)
}
fn retain_paths(&mut self, paths: &HashSet<AbsolutePathBuf>) {
self.values = Arc::new(
self.values
.iter()
.filter(|(path, _)| paths.contains(*path))
.map(|(path, fs)| (path.clone(), Arc::clone(fs)))
.collect(),
);
}
}
impl fmt::Debug for SkillFileSystemsByPath {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("SkillFileSystemsByPath")
.field("len", &self.values.len())
.finish()
}
}
pub fn filter_skill_load_outcome_for_product(
mut outcome: SkillLoadOutcome,
restriction_product: Option<Product>,
) -> SkillLoadOutcome {
outcome
.skills
.retain(|skill| skill.matches_product_restriction_for_product(restriction_product));
let retained_paths: HashSet<AbsolutePathBuf> = outcome
.skills
.iter()
.map(|skill| skill.path_to_skills_md.clone())
.collect();
outcome
.file_systems_by_skill_path
.retain_paths(&retained_paths);
outcome.implicit_skills_by_scripts_dir = Arc::new(
outcome
.implicit_skills_by_scripts_dir
.iter()
.filter(|(_, skill)| skill.matches_product_restriction_for_product(restriction_product))
.map(|(path, skill)| (path.clone(), skill.clone()))
.collect(),
);
outcome.implicit_skills_by_doc_path = Arc::new(
outcome
.implicit_skills_by_doc_path
.iter()
.filter(|(_, skill)| skill.matches_product_restriction_for_product(restriction_product))
.map(|(path, skill)| (path.clone(), skill.clone()))
.collect(),
);
outcome
}

View file

@ -1,270 +0,0 @@
use anyhow::Context;
use anyhow::Result;
use serde::Deserialize;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use codex_login::CodexAuth;
use codex_login::default_client::build_reqwest_client;
const REMOTE_SKILLS_API_TIMEOUT: Duration = Duration::from_secs(30);
// Low-level client for the remote skill API. This is intentionally kept around for
// future wiring, but it is not used yet by any active product surface.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RemoteSkillScope {
WorkspaceShared,
AllShared,
Personal,
Example,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RemoteSkillProductSurface {
Chatgpt,
Codex,
Api,
Atlas,
}
fn as_query_scope(scope: RemoteSkillScope) -> Option<&'static str> {
match scope {
RemoteSkillScope::WorkspaceShared => Some("workspace-shared"),
RemoteSkillScope::AllShared => Some("all-shared"),
RemoteSkillScope::Personal => Some("personal"),
RemoteSkillScope::Example => Some("example"),
}
}
fn as_query_product_surface(product_surface: RemoteSkillProductSurface) -> &'static str {
match product_surface {
RemoteSkillProductSurface::Chatgpt => "chatgpt",
RemoteSkillProductSurface::Codex => "codex",
RemoteSkillProductSurface::Api => "api",
RemoteSkillProductSurface::Atlas => "atlas",
}
}
fn ensure_chatgpt_auth(auth: Option<&CodexAuth>) -> Result<&CodexAuth> {
let Some(auth) = auth else {
anyhow::bail!("chatgpt authentication required for remote skill scopes");
};
if !auth.is_chatgpt_auth() {
anyhow::bail!(
"chatgpt authentication required for remote skill scopes; api key auth is not supported"
);
}
Ok(auth)
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RemoteSkillSummary {
pub id: String,
pub name: String,
pub description: String,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RemoteSkillDownloadResult {
pub id: String,
pub path: PathBuf,
}
#[derive(Debug, Deserialize)]
struct RemoteSkillsResponse {
#[serde(rename = "hazelnuts")]
skills: Vec<RemoteSkill>,
}
#[derive(Debug, Deserialize)]
struct RemoteSkill {
id: String,
name: String,
description: String,
}
pub async fn list_remote_skills(
chatgpt_base_url: String,
auth: Option<&CodexAuth>,
scope: RemoteSkillScope,
product_surface: RemoteSkillProductSurface,
enabled: Option<bool>,
) -> Result<Vec<RemoteSkillSummary>> {
let base_url = chatgpt_base_url.trim_end_matches('/');
let auth = ensure_chatgpt_auth(auth)?;
let url = format!("{base_url}/hazelnuts");
let product_surface = as_query_product_surface(product_surface);
let mut query_params = vec![("product_surface", product_surface)];
if let Some(scope) = as_query_scope(scope) {
query_params.push(("scope", scope));
}
if let Some(enabled) = enabled {
let enabled = if enabled { "true" } else { "false" };
query_params.push(("enabled", enabled));
}
let client = build_reqwest_client();
let mut request = client
.get(&url)
.timeout(REMOTE_SKILLS_API_TIMEOUT)
.query(&query_params);
let token = auth
.get_token()
.context("Failed to read auth token for remote skills")?;
request = request.bearer_auth(token);
if let Some(account_id) = auth.get_account_id() {
request = request.header("chatgpt-account-id", account_id);
}
let response = request
.send()
.await
.with_context(|| format!("Failed to send request to {url}"))?;
let status = response.status();
let body = response.text().await.unwrap_or_default();
if !status.is_success() {
anyhow::bail!("Request failed with status {status} from {url}: {body}");
}
let parsed: RemoteSkillsResponse =
serde_json::from_str(&body).context("Failed to parse skills response")?;
Ok(parsed
.skills
.into_iter()
.map(|skill| RemoteSkillSummary {
id: skill.id,
name: skill.name,
description: skill.description,
})
.collect())
}
pub async fn export_remote_skill(
chatgpt_base_url: String,
codex_home: PathBuf,
auth: Option<&CodexAuth>,
skill_id: &str,
) -> Result<RemoteSkillDownloadResult> {
let auth = ensure_chatgpt_auth(auth)?;
let client = build_reqwest_client();
let base_url = chatgpt_base_url.trim_end_matches('/');
let url = format!("{base_url}/hazelnuts/{skill_id}/export");
let mut request = client.get(&url).timeout(REMOTE_SKILLS_API_TIMEOUT);
let token = auth
.get_token()
.context("Failed to read auth token for remote skills")?;
request = request.bearer_auth(token);
if let Some(account_id) = auth.get_account_id() {
request = request.header("chatgpt-account-id", account_id);
}
let response = request
.send()
.await
.with_context(|| format!("Failed to send download request to {url}"))?;
let status = response.status();
let body = response.bytes().await.context("Failed to read download")?;
if !status.is_success() {
let body_text = String::from_utf8_lossy(&body);
anyhow::bail!("Download failed with status {status} from {url}: {body_text}");
}
if !is_zip_payload(&body) {
anyhow::bail!("Downloaded remote skill payload is not a zip archive");
}
let output_dir = codex_home.join("skills").join(skill_id);
tokio::fs::create_dir_all(&output_dir)
.await
.context("Failed to create downloaded skills directory")?;
let zip_bytes = body.to_vec();
let output_dir_clone = output_dir.clone();
let prefix_candidates = vec![skill_id.to_string()];
tokio::task::spawn_blocking(move || {
extract_zip_to_dir(zip_bytes, &output_dir_clone, &prefix_candidates)
})
.await
.context("Zip extraction task failed")??;
Ok(RemoteSkillDownloadResult {
id: skill_id.to_string(),
path: output_dir,
})
}
fn safe_join(base: &Path, name: &str) -> Result<PathBuf> {
let path = Path::new(name);
for component in path.components() {
match component {
Component::Normal(_) => {}
_ => {
anyhow::bail!("Invalid file path in remote skill payload: {name}");
}
}
}
Ok(base.join(path))
}
fn is_zip_payload(bytes: &[u8]) -> bool {
bytes.starts_with(b"PK\x03\x04")
|| bytes.starts_with(b"PK\x05\x06")
|| bytes.starts_with(b"PK\x07\x08")
}
fn extract_zip_to_dir(
bytes: Vec<u8>,
output_dir: &Path,
prefix_candidates: &[String],
) -> Result<()> {
let cursor = std::io::Cursor::new(bytes);
let mut archive = zip::ZipArchive::new(cursor).context("Failed to open zip archive")?;
for i in 0..archive.len() {
let mut file = archive.by_index(i).context("Failed to read zip entry")?;
if file.is_dir() {
continue;
}
let raw_name = file.name().to_string();
let normalized = normalize_zip_name(&raw_name, prefix_candidates);
let Some(normalized) = normalized else {
continue;
};
let file_path = safe_join(output_dir, &normalized)?;
if let Some(parent) = file_path.parent() {
std::fs::create_dir_all(parent)
.with_context(|| format!("Failed to create parent dir for {normalized}"))?;
}
let mut out = std::fs::File::create(&file_path)
.with_context(|| format!("Failed to create file {normalized}"))?;
std::io::copy(&mut file, &mut out)
.with_context(|| format!("Failed to write skill file {normalized}"))?;
}
Ok(())
}
fn normalize_zip_name(name: &str, prefix_candidates: &[String]) -> Option<String> {
let mut trimmed = name.trim_start_matches("./");
for prefix in prefix_candidates {
if prefix.is_empty() {
continue;
}
let prefix = format!("{prefix}/");
if let Some(rest) = trimmed.strip_prefix(&prefix) {
trimmed = rest;
break;
}
}
if trimmed.is_empty() {
None
} else {
Some(trimmed.to_string())
}
}

View file

@ -1,755 +0,0 @@
use crate::model::SkillMetadata;
use codex_otel::SessionTelemetry;
use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC;
use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC;
use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC;
use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC;
use codex_protocol::protocol::SkillScope;
use codex_utils_output_truncation::approx_token_count;
const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000;
const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2;
const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 10;
const APPROX_BYTES_PER_TOKEN: usize = 4;
pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX: &str = "Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of";
pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str =
"Warning: Exceeded skills context budget. All skill descriptions were removed and";
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum SkillMetadataBudget {
Tokens(usize),
Characters(usize),
}
impl SkillMetadataBudget {
fn limit(self) -> usize {
match self {
Self::Tokens(limit) | Self::Characters(limit) => limit,
}
}
fn cost(self, text: &str) -> usize {
match self {
Self::Tokens(_) => approx_token_count(text),
Self::Characters(_) => text.chars().count(),
}
}
fn cost_from_counts(self, chars: usize, bytes: usize) -> usize {
match self {
Self::Tokens(_) => approx_token_count_from_bytes(bytes),
Self::Characters(_) => chars,
}
}
}
fn approx_token_count_from_bytes(bytes: usize) -> usize {
bytes.saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) / APPROX_BYTES_PER_TOKEN
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillRenderReport {
pub total_count: usize,
pub included_count: usize,
pub omitted_count: usize,
pub truncated_description_chars: usize,
pub truncated_description_count: usize,
}
#[derive(Clone, Copy)]
pub enum SkillRenderSideEffects<'a> {
None,
ThreadStart {
session_telemetry: &'a SessionTelemetry,
},
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AvailableSkills {
pub skill_lines: Vec<String>,
pub report: SkillRenderReport,
pub warning_message: Option<String>,
}
pub fn default_skill_metadata_budget(context_window: Option<i64>) -> SkillMetadataBudget {
context_window
.and_then(|window| usize::try_from(window).ok())
.filter(|window| *window > 0)
.map(|window| {
SkillMetadataBudget::Tokens(
window
.saturating_mul(SKILL_METADATA_CONTEXT_WINDOW_PERCENT)
.saturating_div(100)
.max(1),
)
})
.unwrap_or(SkillMetadataBudget::Characters(
DEFAULT_SKILL_METADATA_CHAR_BUDGET,
))
}
pub fn build_available_skills(
skills: &[SkillMetadata],
budget: SkillMetadataBudget,
side_effects: SkillRenderSideEffects<'_>,
) -> Option<AvailableSkills> {
if skills.is_empty() {
record_skill_render_side_effects(
side_effects,
/*total_count*/ 0,
/*included_count*/ 0,
/*omitted_count*/ 0,
/*truncated_description_chars*/ 0,
);
return None;
}
let (skill_lines, report) = render_skill_lines(skills, budget);
let warning_message = if report.omitted_count > 0 {
let skill_word = if report.omitted_count == 1 {
"skill"
} else {
"skills"
};
let verb = if report.omitted_count == 1 {
"was"
} else {
"were"
};
Some(format!(
"{} {} additional {} {} not included in the model-visible skills list.",
budget_warning_prefix(budget, SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX),
report.omitted_count,
skill_word,
verb
))
} else if report.average_truncated_description_chars()
> SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS
{
Some(format!(
"{} {} characters per skill.",
budget_warning_prefix(budget, SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX),
report.average_truncated_description_chars()
))
} else {
None
};
record_skill_render_side_effects(
side_effects,
report.total_count,
report.included_count,
report.omitted_count,
report.truncated_description_chars,
);
if report.omitted_count > 0 || report.truncated_description_chars > 0 {
tracing::info!(
budget_limit = budget.limit(),
total_skills = report.total_count,
included_skills = report.included_count,
omitted_skills = report.omitted_count,
truncated_description_chars_per_skill = report.average_truncated_description_chars(),
truncated_skill_descriptions = report.truncated_description_count,
"truncated skill metadata to fit skills context budget"
);
}
Some(AvailableSkills {
skill_lines,
report,
warning_message,
})
}
fn budget_warning_prefix(budget: SkillMetadataBudget, prefix: &str) -> String {
match budget {
SkillMetadataBudget::Tokens(_) => prefix.replacen(
"Exceeded skills context budget.",
"Exceeded skills context budget of 2%.",
1,
),
SkillMetadataBudget::Characters(_) => prefix.to_string(),
}
}
fn record_skill_render_side_effects(
side_effects: SkillRenderSideEffects<'_>,
total_count: usize,
included_count: usize,
omitted_count: usize,
truncated_description_chars: usize,
) {
match side_effects {
SkillRenderSideEffects::None => {}
SkillRenderSideEffects::ThreadStart { session_telemetry } => {
session_telemetry.histogram(
THREAD_SKILLS_ENABLED_TOTAL_METRIC,
i64::try_from(total_count).unwrap_or(i64::MAX),
&[],
);
session_telemetry.histogram(
THREAD_SKILLS_KEPT_TOTAL_METRIC,
i64::try_from(included_count).unwrap_or(i64::MAX),
&[],
);
session_telemetry.histogram(
THREAD_SKILLS_TRUNCATED_METRIC,
if omitted_count > 0 { 1 } else { 0 },
&[],
);
session_telemetry.histogram(
THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC,
i64::try_from(truncated_description_chars).unwrap_or(i64::MAX),
&[],
);
}
}
}
fn render_skill_lines(
skills: &[SkillMetadata],
budget: SkillMetadataBudget,
) -> (Vec<String>, SkillRenderReport) {
let ordered_skills = ordered_skills_for_budget(skills);
let skill_lines = ordered_skills
.into_iter()
.map(SkillLine::new)
.collect::<Vec<_>>();
let full_cost = skill_lines.iter().fold(0usize, |used, line| {
used.saturating_add(line.full_cost(budget))
});
if full_cost <= budget.limit() {
let included = skill_lines
.iter()
.map(SkillLine::render_full)
.collect::<Vec<_>>();
return (
included,
skill_render_report(
/*total_count*/ skills.len(),
/*included_count*/ skill_lines.len(),
/*omitted_count*/ 0,
/*truncated_description_chars*/ 0,
/*truncated_description_count*/ 0,
),
);
}
let minimum_cost = skill_lines.iter().fold(0usize, |used, line| {
used.saturating_add(line.minimum_cost(budget))
});
if minimum_cost <= budget.limit() {
let rendered = render_lines_with_description_budget(
budget,
&skill_lines,
budget.limit().saturating_sub(minimum_cost),
);
let (truncated_description_chars, truncated_description_count) =
sum_description_truncation(&rendered);
let included = rendered
.into_iter()
.map(|rendered| rendered.line)
.collect::<Vec<_>>();
return (
included,
skill_render_report(
/*total_count*/ skills.len(),
/*included_count*/ skill_lines.len(),
/*omitted_count*/ 0,
truncated_description_chars,
truncated_description_count,
),
);
}
render_minimum_skill_lines_until_budget(budget, skill_lines, skills.len())
}
fn render_minimum_skill_lines_until_budget(
budget: SkillMetadataBudget,
skill_lines: Vec<SkillLine<'_>>,
total_count: usize,
) -> (Vec<String>, SkillRenderReport) {
let mut included = Vec::new();
let mut used = 0usize;
let mut omitted_count = 0usize;
let mut truncated_description_chars = 0usize;
let mut truncated_description_count = 0usize;
for line in skill_lines {
let line_cost = line.minimum_cost(budget);
let description_char_count = line.description_char_count();
if used.saturating_add(line_cost) <= budget.limit() {
used = used.saturating_add(line_cost);
included.push(line.render_minimum());
} else {
omitted_count = omitted_count.saturating_add(1);
}
truncated_description_chars =
truncated_description_chars.saturating_add(description_char_count);
if description_char_count > 0 {
truncated_description_count = truncated_description_count.saturating_add(1);
}
}
let report = skill_render_report(
total_count,
included.len(),
omitted_count,
truncated_description_chars,
truncated_description_count,
);
(included, report)
}
fn skill_render_report(
total_count: usize,
included_count: usize,
omitted_count: usize,
truncated_description_chars: usize,
truncated_description_count: usize,
) -> SkillRenderReport {
SkillRenderReport {
total_count,
included_count,
omitted_count,
truncated_description_chars,
truncated_description_count,
}
}
impl SkillRenderReport {
fn average_truncated_description_chars(&self) -> usize {
if self.truncated_description_count == 0 {
return 0;
}
self.truncated_description_chars
.saturating_add(self.truncated_description_count.saturating_sub(1))
/ self.truncated_description_count
}
}
struct SkillLine<'a> {
name: &'a str,
description: &'a str,
path: String,
}
struct RenderedSkillLine {
line: String,
truncated_chars: usize,
}
struct DescriptionBudgetLine<'a> {
line: &'a SkillLine<'a>,
description_char_count: usize,
extra_costs: Vec<usize>,
}
fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize) {
rendered
.iter()
.fold((0usize, 0usize), |(chars, count), line| {
if line.truncated_chars == 0 {
(chars, count)
} else {
(
chars.saturating_add(line.truncated_chars),
count.saturating_add(1),
)
}
})
}
impl<'a> SkillLine<'a> {
fn new(skill: &'a SkillMetadata) -> Self {
Self {
name: skill.name.as_str(),
description: skill.description.as_str(),
path: skill.path_to_skills_md.to_string_lossy().replace('\\', "/"),
}
}
fn full_cost(&self, budget: SkillMetadataBudget) -> usize {
line_cost(budget, &self.render_full())
}
fn minimum_cost(&self, budget: SkillMetadataBudget) -> usize {
line_cost(budget, &self.render_minimum())
}
fn description_char_count(&self) -> usize {
self.description.chars().count()
}
fn render_full(&self) -> String {
self.render_with_description(self.description)
}
fn render_minimum(&self) -> String {
self.render_with_description("")
}
fn rendered_description_prefix_len(&self, description_chars: usize) -> usize {
self.description
.char_indices()
.nth(description_chars)
.map_or(self.description.len(), |(idx, _)| idx)
}
fn render_with_description_chars(&self, description_chars: usize) -> String {
if description_chars == 0 {
format!("- {}: (file: {})", self.name, self.path)
} else {
let end = self.rendered_description_prefix_len(description_chars);
let description = &self.description[..end];
format!("- {}: {} (file: {})", self.name, description, self.path)
}
}
fn render_with_description(&self, description: &str) -> String {
if description.is_empty() {
format!("- {}: (file: {})", self.name, self.path)
} else {
format!("- {}: {} (file: {})", self.name, description, self.path)
}
}
}
impl<'a> DescriptionBudgetLine<'a> {
fn new(line: &'a SkillLine<'a>, budget: SkillMetadataBudget) -> Self {
let minimum_line = line.render_minimum();
let minimum_chars = minimum_line.chars().count().saturating_add(1);
let minimum_bytes = minimum_line.len().saturating_add(1);
let minimum_cost = budget.cost_from_counts(minimum_chars, minimum_bytes);
let description_char_count = line.description_char_count();
let mut extra_costs = Vec::with_capacity(description_char_count.saturating_add(1));
extra_costs.push(0);
let mut prefix_chars = 0usize;
let mut prefix_bytes = 0usize;
for ch in line.description.chars() {
prefix_chars = prefix_chars.saturating_add(1);
prefix_bytes = prefix_bytes.saturating_add(ch.len_utf8());
let rendered_chars = minimum_chars.saturating_add(prefix_chars).saturating_add(1);
let rendered_bytes = minimum_bytes.saturating_add(prefix_bytes).saturating_add(1);
let cost = budget
.cost_from_counts(rendered_chars, rendered_bytes)
.saturating_sub(minimum_cost);
extra_costs.push(cost);
}
Self {
line,
description_char_count,
extra_costs,
}
}
}
fn line_cost(budget: SkillMetadataBudget, line: &str) -> usize {
budget.cost(&format!("{line}\n"))
}
fn render_lines_with_description_budget(
budget: SkillMetadataBudget,
skill_lines: &[SkillLine<'_>],
limit: usize,
) -> Vec<RenderedSkillLine> {
let budget_lines = skill_lines
.iter()
.map(|line| DescriptionBudgetLine::new(line, budget))
.collect::<Vec<_>>();
let mut char_allocations = vec![0usize; budget_lines.len()];
let mut current_extra_costs = vec![0usize; budget_lines.len()];
let mut remaining = limit;
// Distribute description space one character at a time across skills.
// Short descriptions naturally drop out, so their unused share can go to
// longer descriptions instead of being stranded in a fixed per-skill quota.
loop {
let mut changed = false;
for (index, line) in budget_lines.iter().enumerate() {
if char_allocations[index] >= line.description_char_count {
continue;
}
let current_cost = current_extra_costs[index];
let next_chars = char_allocations[index].saturating_add(1);
let next_cost = line.extra_costs[next_chars];
let delta = next_cost.saturating_sub(current_cost);
if delta <= remaining {
char_allocations[index] = next_chars;
current_extra_costs[index] = next_cost;
remaining = remaining.saturating_sub(delta);
changed = true;
}
}
if !changed {
break;
}
}
budget_lines
.iter()
.zip(char_allocations)
.map(|(line, description_chars)| {
let truncated_chars = line
.description_char_count
.saturating_sub(description_chars);
RenderedSkillLine {
line: line.line.render_with_description_chars(description_chars),
truncated_chars,
}
})
.collect()
}
fn ordered_skills_for_budget(skills: &[SkillMetadata]) -> Vec<&SkillMetadata> {
let mut ordered = skills.iter().collect::<Vec<_>>();
ordered.sort_by(|a, b| {
prompt_scope_rank(a.scope)
.cmp(&prompt_scope_rank(b.scope))
.then_with(|| a.name.cmp(&b.name))
.then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md))
});
ordered
}
fn prompt_scope_rank(scope: SkillScope) -> u8 {
match scope {
SkillScope::System => 0,
SkillScope::Admin => 1,
SkillScope::Repo => 2,
SkillScope::User => 3,
}
}
#[cfg(test)]
mod tests {
use super::*;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
fn make_skill(name: &str, scope: SkillScope) -> SkillMetadata {
SkillMetadata {
name: name.to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(),
scope,
}
}
fn make_skill_with_description(
name: &str,
scope: SkillScope,
description: &str,
) -> SkillMetadata {
let mut skill = make_skill(name, scope);
skill.description = description.to_string();
skill
}
fn expected_skill_line(skill: &SkillMetadata, description: &str) -> String {
SkillLine::new(skill).render_with_description(description)
}
#[test]
fn default_budget_uses_two_percent_of_full_context_window() {
assert_eq!(
default_skill_metadata_budget(Some(200_000)),
SkillMetadataBudget::Tokens(4_000)
);
assert_eq!(
default_skill_metadata_budget(Some(99)),
SkillMetadataBudget::Tokens(1)
);
}
#[test]
fn default_budget_falls_back_to_characters_without_context_window() {
assert_eq!(
default_skill_metadata_budget(/*context_window*/ None),
SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET)
);
assert_eq!(
default_skill_metadata_budget(Some(-1)),
SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET)
);
}
#[test]
fn budgeted_rendering_truncates_descriptions_equally_before_omitting_skills() {
let alpha = make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdef");
let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyz");
let minimum_cost = SkillLine::new(&alpha)
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
let rendered = build_available_skills(
&[beta.clone(), alpha.clone()],
budget,
SkillRenderSideEffects::None,
)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(rendered.report.truncated_description_chars, 8);
assert_eq!(rendered.warning_message, None);
assert_eq!(
rendered.skill_lines,
vec![
expected_skill_line(&alpha, "ab"),
expected_skill_line(&beta, "uv"),
]
);
}
#[test]
fn budgeted_rendering_does_not_warn_when_average_description_truncation_is_within_threshold() {
let alpha = make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdefghij");
let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyzabcd");
let minimum_cost = SkillLine::new(&alpha)
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
let rendered = build_available_skills(&[alpha, beta], budget, SkillRenderSideEffects::None)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(rendered.report.truncated_description_chars, 16);
assert_eq!(rendered.report.truncated_description_count, 2);
assert_eq!(rendered.warning_message, None);
}
#[test]
fn budgeted_rendering_warns_when_average_description_truncation_exceeds_threshold() {
let alpha =
make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdefghijklmnop");
let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyzabcdefghij");
let minimum_cost = SkillLine::new(&alpha)
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
let rendered = build_available_skills(&[alpha, beta], budget, SkillRenderSideEffects::None)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(rendered.report.truncated_description_chars, 28);
assert_eq!(rendered.report.truncated_description_count, 2);
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of 14 characters per skill."
.to_string()
)
);
}
#[test]
fn budgeted_rendering_redistributes_unused_description_budget() {
let short = make_skill_with_description("short-skill", SkillScope::Repo, "x");
let long = make_skill_with_description("long-skill", SkillScope::Repo, "abcdefghi");
let minimum_cost = SkillLine::new(&short)
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))
+ SkillLine::new(&long).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 11);
let rendered = build_available_skills(
&[short.clone(), long.clone()],
budget,
SkillRenderSideEffects::None,
)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(rendered.warning_message, None);
assert_eq!(
rendered.skill_lines,
vec![
expected_skill_line(&long, "abcdefgh"),
expected_skill_line(&short, "x"),
]
);
}
#[test]
fn budgeted_rendering_preserves_prompt_priority_when_minimum_lines_exceed_budget() {
let system = make_skill("system-skill", SkillScope::System);
let user = make_skill("user-skill", SkillScope::User);
let repo = make_skill("repo-skill", SkillScope::Repo);
let admin = make_skill("admin-skill", SkillScope::Admin);
let system_cost = SkillMetadataBudget::Characters(usize::MAX)
.cost(&format!("{}\n", SkillLine::new(&system).render_minimum()));
let admin_cost = SkillMetadataBudget::Characters(usize::MAX)
.cost(&format!("{}\n", SkillLine::new(&admin).render_minimum()));
let budget = SkillMetadataBudget::Characters(system_cost + admin_cost);
let rendered = build_available_skills(
&[system, user, repo, admin],
budget,
SkillRenderSideEffects::None,
)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 2);
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
.to_string()
)
);
let rendered_text = rendered.skill_lines.join("\n");
assert!(rendered_text.contains("- system-skill:"));
assert!(rendered_text.contains("- admin-skill:"));
assert!(!rendered_text.contains("desc"));
assert!(!rendered_text.contains("- repo-skill:"));
assert!(!rendered_text.contains("- user-skill:"));
}
#[test]
fn budgeted_rendering_keeps_scanning_after_oversized_entry() {
let mut oversized = make_skill("oversized-system-skill", SkillScope::System);
oversized.description = "desc ".repeat(100);
let repo = make_skill("repo-skill", SkillScope::Repo);
let repo_cost = SkillMetadataBudget::Characters(usize::MAX)
.cost(&format!("{}\n", SkillLine::new(&repo).render_full()));
let budget = SkillMetadataBudget::Characters(repo_cost);
let rendered =
build_available_skills(&[oversized, repo], budget, SkillRenderSideEffects::None)
.expect("skills render");
assert_eq!(rendered.report.included_count, 1);
assert_eq!(rendered.report.omitted_count, 1);
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
.to_string()
)
);
let rendered_text = rendered.skill_lines.join("\n");
assert!(!rendered_text.contains("- oversized-system-skill:"));
assert!(rendered_text.contains("- repo-skill:"));
}
}

View file

@ -1,15 +0,0 @@
use codex_utils_absolute_path::AbsolutePathBuf;
const SYSTEM_SKILLS_DIR_NAME: &str = ".system";
const SKILLS_DIR_NAME: &str = "skills";
/// Returns the legacy on-disk cache location for previously embedded system skills.
pub(crate) fn system_cache_root_dir(codex_home: &AbsolutePathBuf) -> AbsolutePathBuf {
codex_home
.join(SKILLS_DIR_NAME)
.join(SYSTEM_SKILLS_DIR_NAME)
}
pub(crate) fn uninstall_system_skills(codex_home: &AbsolutePathBuf) {
let _ = std::fs::remove_dir_all(system_cache_root_dir(codex_home));
}

View file

@ -32,7 +32,6 @@ codex-apply-patch = { workspace = true }
codex-async-utils = { workspace = true }
codex-code-mode = { workspace = true }
codex-config = { workspace = true }
codex-core-skills = { workspace = true }
codex-exec-server = { workspace = true }
codex-features = { workspace = true }
codex-feedback = { workspace = true }

View file

@ -79,9 +79,7 @@ pub(crate) async fn run_codex_thread_interactive(
auth_manager,
models_manager,
environment_manager: Arc::clone(&parent_session.services.environment_manager),
skills_manager: Arc::clone(&parent_session.services.skills_manager),
mcp_manager: Arc::clone(&parent_session.services.mcp_manager),
skills_watcher: Arc::clone(&parent_session.services.skills_watcher),
conversation_history: initial_history.unwrap_or(InitialHistory::New),
session_source: SessionSource::SubAgent(subagent_source.clone()),
agent_control: parent_session.services.agent_control.clone(),

View file

@ -2465,10 +2465,6 @@ impl Config {
.network
.is_some()
}
pub fn bundled_skills_enabled(&self) -> bool {
crate::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack)
}
}
pub(crate) fn uses_deprecated_instructions_file(config_layer_stack: &ConfigLayerStack) -> bool {

View file

@ -1,5 +1,3 @@
use codex_core_skills::injection::SkillInjection;
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
@ -9,16 +7,6 @@ pub(crate) struct SkillInstructions {
pub(crate) contents: String,
}
impl From<&SkillInjection> for SkillInstructions {
fn from(skill: &SkillInjection) -> Self {
Self {
name: skill.name.clone(),
path: skill.path.clone(),
contents: skill.contents.clone(),
}
}
}
impl ContextualUserFragment for SkillInstructions {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = "<skill>";

View file

@ -95,11 +95,6 @@ pub mod sandboxing;
mod session_prefix;
mod session_startup_prewarm;
mod shell_detect;
pub mod skills;
pub(crate) use skills::SkillLoadOutcome;
pub(crate) use skills::SkillsManager;
pub(crate) use skills::manager;
mod skills_watcher;
mod stream_events_utils;
pub mod test_support;
mod unified_exec;

View file

@ -187,7 +187,6 @@ pub(crate) use self::session::SessionSettingsUpdate;
use self::turn::AssistantMessageStreamParsers;
use self::turn::realtime_text_for_event;
use self::turn_context::TurnContext;
use self::turn_context::TurnSkillsContext;
#[cfg(test)]
mod rollout_reconstruction_tests;
@ -242,8 +241,6 @@ pub(crate) struct PreviousTurnSettings {
pub(crate) realtime_active: Option<bool>,
}
use crate::SkillLoadOutcome;
use crate::SkillsManager;
use crate::agents_md::AgentsMdManager;
use crate::context::UserInstructions;
use crate::exec_policy::ExecPolicyUpdateError;
@ -259,7 +256,6 @@ use crate::rollout::policy::EventPersistenceMode;
use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
use crate::shell;
use crate::shell_snapshot::ShellSnapshot;
use crate::skills_watcher::SkillsWatcher;
use crate::state::ActiveTurn;
use crate::state::MailboxDeliveryPhase;
use crate::state::PendingRequestPermissions;
@ -364,9 +360,7 @@ pub(crate) struct CodexSpawnArgs {
pub(crate) auth_manager: Arc<AuthManager>,
pub(crate) models_manager: Arc<ModelsManager>,
pub(crate) environment_manager: Arc<EnvironmentManager>,
pub(crate) skills_manager: Arc<SkillsManager>,
pub(crate) mcp_manager: Arc<McpManager>,
pub(crate) skills_watcher: Arc<SkillsWatcher>,
pub(crate) conversation_history: InitialHistory,
pub(crate) session_source: SessionSource,
pub(crate) agent_control: AgentControl,
@ -419,9 +413,7 @@ impl Codex {
auth_manager,
models_manager,
environment_manager,
skills_manager,
mcp_manager,
skills_watcher,
conversation_history,
session_source,
agent_control,
@ -604,9 +596,7 @@ impl Codex {
agent_status_tx.clone(),
conversation_history,
session_source_clone,
skills_manager,
mcp_manager.clone(),
skills_watcher,
agent_control,
environment_manager,
analytics_events_client,

View file

@ -138,7 +138,6 @@ pub(super) async fn spawn_review_thread(
dynamic_tools: parent_turn_context.dynamic_tools.clone(),
truncation_policy: model_info.truncation_policy.into(),
turn_metadata_state,
turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.outcome.clone()),
turn_timing_state: Arc::new(TurnTimingState::default()),
};

View file

@ -265,9 +265,7 @@ impl Session {
agent_status: watch::Sender<AgentStatus>,
initial_history: InitialHistory,
session_source: SessionSource,
skills_manager: Arc<SkillsManager>,
mcp_manager: Arc<McpManager>,
skills_watcher: Arc<SkillsWatcher>,
agent_control: AgentControl,
environment_manager: Arc<EnvironmentManager>,
analytics_events_client: Option<AnalyticsEventsClient>,
@ -731,9 +729,7 @@ impl Session {
guardian_rejections: Mutex::new(HashMap::new()),
guardian_rejection_circuit_breaker: Mutex::new(Default::default()),
runtime_handle: tokio::runtime::Handle::current(),
skills_manager,
mcp_manager: Arc::clone(&mcp_manager),
skills_watcher,
agent_control,
network_proxy,
network_approval: Arc::clone(&network_approval),

View file

@ -14,13 +14,8 @@ use crate::context::TurnAborted;
use crate::exec::ExecCapturePolicy;
use crate::function_tool::FunctionCallError;
use crate::shell::default_user_shell;
use crate::skills::SkillRenderSideEffects;
use crate::skills::build_available_skills;
use crate::skills::render::SkillMetadataBudget;
use crate::tools::format_exec_output_str;
use codex_core_skills::SkillMetadata;
use codex_core_skills::SkillsLoadInput;
use codex_features::Feature;
use codex_features::Features;
use codex_login::CodexAuth;
@ -68,12 +63,6 @@ use codex_execpolicy::Decision;
use codex_execpolicy::NetworkRuleProtocol;
use codex_execpolicy::Policy;
use codex_network_proxy::NetworkProxyConfig;
use codex_otel::MetricsClient;
use codex_otel::MetricsConfig;
use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC;
use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC;
use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC;
use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC;
use codex_otel::TelemetryAuthMode;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::ModeKind;
@ -116,7 +105,6 @@ use codex_protocol::protocol::RealtimeVoice;
use codex_protocol::protocol::RealtimeVoicesList;
use codex_protocol::protocol::ResumedHistory;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::SkillScope;
use codex_protocol::protocol::Submission;
use codex_protocol::protocol::ThreadRolledBackEvent;
use codex_protocol::protocol::TokenCountEvent;
@ -138,16 +126,10 @@ use core_test_support::responses::mount_sse_once;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::test_codex::test_codex;
use core_test_support::test_path_buf;
use core_test_support::tracing::install_test_tracing;
use core_test_support::wait_for_event;
use opentelemetry::trace::TraceContextExt;
use opentelemetry::trace::TraceId;
use opentelemetry_sdk::metrics::InMemoryMetricExporter;
use opentelemetry_sdk::metrics::data::AggregatedMetrics;
use opentelemetry_sdk::metrics::data::Metric;
use opentelemetry_sdk::metrics::data::MetricData;
use opentelemetry_sdk::metrics::data::ResourceMetrics;
use std::path::Path;
use std::time::Duration;
use tokio::sync::Semaphore;
@ -166,18 +148,6 @@ use std::time::Duration as StdDuration;
mod guardian_tests;
fn skills_load_input_from_config(
config: &Config,
effective_skill_roots: Vec<codex_utils_absolute_path::AbsolutePathBuf>,
) -> SkillsLoadInput {
SkillsLoadInput::new(
config.cwd.clone(),
effective_skill_roots,
config.config_layer_stack.clone(),
config.bundled_skills_enabled(),
)
}
fn sample_epiphany_state_for_persistence() -> EpiphanyThreadState {
EpiphanyThreadState {
revision: 1,
@ -468,54 +438,6 @@ fn assistant_message(text: &str) -> ResponseItem {
}
}
fn test_session_telemetry_without_metadata() -> SessionTelemetry {
let exporter = InMemoryMetricExporter::default();
let metrics = MetricsClient::new(
MetricsConfig::in_memory("test", "codex-core", env!("CARGO_PKG_VERSION"), exporter)
.with_runtime_reader(),
)
.expect("in-memory metrics client");
SessionTelemetry::new(
ThreadId::new(),
"gpt-5.4",
"gpt-5.4",
/*account_id*/ None,
/*account_email*/ None,
/*auth_mode*/ None,
"test_originator".to_string(),
/*log_user_prompts*/ false,
"tty".to_string(),
SessionSource::Cli,
)
.with_metrics_without_metadata_tags(metrics)
}
fn find_metric<'a>(resource_metrics: &'a ResourceMetrics, name: &str) -> &'a Metric {
for scope_metrics in resource_metrics.scope_metrics() {
for metric in scope_metrics.metrics() {
if metric.name() == name {
return metric;
}
}
}
panic!("metric {name} missing");
}
fn histogram_sum(resource_metrics: &ResourceMetrics, name: &str) -> u64 {
let metric = find_metric(resource_metrics, name);
match metric.data() {
AggregatedMetrics::F64(data) => match data {
MetricData::Histogram(histogram) => {
let points: Vec<_> = histogram.data_points().collect();
assert_eq!(points.len(), 1);
points[0].sum().round() as u64
}
_ => panic!("unexpected histogram aggregation"),
},
_ => panic!("unexpected metric data type"),
}
}
#[tokio::test]
async fn regular_turn_emits_turn_started_without_waiting_for_startup_prewarm() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
@ -2891,92 +2813,6 @@ async fn session_configuration_apply_permission_profile_preserves_existing_deny_
);
}
#[cfg_attr(windows, ignore)]
#[tokio::test]
async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
let (session, _turn_context) = make_session_and_context().await;
let parent_config = session.get_config().await;
let codex_home = parent_config.codex_home.clone();
let skill_dir = codex_home.join("skills").join("demo");
std::fs::create_dir_all(&skill_dir).expect("create skill dir");
let skill_path = skill_dir.join("SKILL.md");
std::fs::write(
&skill_path,
"---\nname: demo-skill\ndescription: demo description\n---\n\n# Body\n",
)
.expect("write skill");
let skill_fs = session
.services
.environment_manager
.default_environment()
.map(|environment| environment.get_filesystem())
.unwrap_or_else(|| std::sync::Arc::clone(&codex_exec_server::LOCAL_FS));
let parent_outcome = session
.services
.skills_manager
.skills_for_cwd(
&skills_load_input_from_config(&parent_config, Vec::new()),
/*force_reload*/ true,
Some(Arc::clone(&skill_fs)),
)
.await;
let parent_skill = parent_outcome
.skills
.iter()
.find(|skill| skill.name == "demo-skill")
.expect("demo skill should be discovered");
assert_eq!(parent_outcome.is_skill_enabled(parent_skill), true);
let role_path = codex_home.join("skills-role.toml");
std::fs::write(
&role_path,
format!(
r#"developer_instructions = "Stay focused"
[[skills.config]]
path = "{}"
enabled = false
"#,
skill_path.display()
),
)
.expect("write role config");
let mut child_config = (*parent_config).clone();
child_config.agent_roles.insert(
"custom".to_string(),
crate::config::AgentRoleConfig {
description: None,
config_file: Some(role_path.to_path_buf()),
nickname_candidates: None,
},
);
crate::agent::role::apply_role_to_config(&mut child_config, Some("custom"))
.await
.expect("custom role should apply");
{
let mut state = session.state.lock().await;
state.session_configuration.original_config_do_not_use = Arc::new(child_config);
}
let child_turn = session
.new_default_turn_with_sub_id("role-skill-turn".to_string())
.await;
let child_skill = child_turn
.turn_skills
.outcome
.skills
.iter()
.find(|skill| skill.name == "demo-skill")
.expect("demo skill should be discovered");
assert_eq!(
child_turn.turn_skills.outcome.is_skill_enabled(child_skill),
false
);
}
#[tokio::test]
async fn session_configuration_apply_rederives_legacy_file_system_policy_on_cwd_update() {
let mut session_configuration = make_session_configuration_for_tests().await;
@ -3115,10 +2951,6 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
let (tx_event, _rx_event) = async_channel::unbounded();
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let mcp_manager = Arc::new(McpManager::new());
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
/*bundled_skills_enabled*/ true,
));
let result = Session::new(
session_configuration,
Arc::clone(&config),
@ -3129,9 +2961,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
agent_status_tx,
InitialHistory::New,
SessionSource::Exec,
skills_manager,
mcp_manager,
Arc::new(SkillsWatcher::noop()),
AgentControl::default(),
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
/*analytics_events_client*/ None,
@ -3225,17 +3055,11 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
let state = SessionState::new(session_configuration.clone());
let mcp_manager = Arc::new(McpManager::new());
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
/*bundled_skills_enabled*/ true,
));
let network_approval = Arc::new(NetworkApprovalService::default());
let environment = Arc::new(
codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None)
.expect("create environment"),
);
let skills_watcher = Arc::new(SkillsWatcher::noop());
let services = SessionServices {
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::new_uninitialized(
&config.permissions.approval_policy,
@ -3269,9 +3093,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
guardian_rejections: Mutex::new(std::collections::HashMap::new()),
guardian_rejection_circuit_breaker: Mutex::new(Default::default()),
runtime_handle: tokio::runtime::Handle::current(),
skills_manager,
mcp_manager,
skills_watcher,
agent_control,
network_proxy: None,
network_approval: Arc::clone(&network_approval),
@ -3299,15 +3121,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
config.js_repl_node_path.clone(),
config.js_repl_node_module_dirs.clone(),
));
let skills_input = skills_load_input_from_config(&per_turn_config, Vec::new());
let skill_fs = environment.get_filesystem();
let skills_outcome = Arc::new(
services
.skills_manager
.skills_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.await,
);
let turn_context = Session::make_turn_context(
conversation_id,
Some(Arc::clone(&auth_manager)),
@ -3326,7 +3139,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
session_configuration.cwd.clone(),
"turn_id".to_string(),
Arc::clone(&js_repl),
skills_outcome,
);
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
@ -3423,10 +3235,6 @@ async fn make_session_with_config_and_rx(
let (tx_event, rx_event) = async_channel::unbounded();
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let mcp_manager = Arc::new(McpManager::new());
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
/*bundled_skills_enabled*/ true,
));
let session = Session::new(
session_configuration,
@ -3438,9 +3246,7 @@ async fn make_session_with_config_and_rx(
agent_status_tx,
InitialHistory::New,
SessionSource::Exec,
skills_manager,
mcp_manager,
Arc::new(SkillsWatcher::noop()),
AgentControl::default(),
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
/*analytics_events_client*/ None,
@ -4527,17 +4333,11 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
let state = SessionState::new(session_configuration.clone());
let mcp_manager = Arc::new(McpManager::new());
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
/*bundled_skills_enabled*/ true,
));
let network_approval = Arc::new(NetworkApprovalService::default());
let environment = Arc::new(
codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None)
.expect("create environment"),
);
let skills_watcher = Arc::new(SkillsWatcher::noop());
let services = SessionServices {
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::new_uninitialized(
&config.permissions.approval_policy,
@ -4571,9 +4371,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
guardian_rejections: Mutex::new(std::collections::HashMap::new()),
guardian_rejection_circuit_breaker: Mutex::new(Default::default()),
runtime_handle: tokio::runtime::Handle::current(),
skills_manager,
mcp_manager,
skills_watcher,
agent_control,
network_proxy: None,
network_approval: Arc::clone(&network_approval),
@ -4601,15 +4399,6 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
config.js_repl_node_path.clone(),
config.js_repl_node_module_dirs.clone(),
));
let skills_input = skills_load_input_from_config(&per_turn_config, Vec::new());
let skill_fs = environment.get_filesystem();
let skills_outcome = Arc::new(
services
.skills_manager
.skills_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.await,
);
let turn_context = Arc::new(Session::make_turn_context(
conversation_id,
Some(Arc::clone(&auth_manager)),
@ -4628,7 +4417,6 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
session_configuration.cwd.clone(),
"turn_id".to_string(),
Arc::clone(&js_repl),
skills_outcome,
));
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
@ -5034,200 +4822,6 @@ async fn build_initial_context_omits_default_image_save_location_without_image_h
);
}
#[tokio::test]
async fn build_initial_context_trims_skill_metadata_from_context_window_budget() {
let (session, mut turn_context) = make_session_and_context().await;
let mut outcome = SkillLoadOutcome::default();
outcome.skills = vec![
SkillMetadata {
name: "admin-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(),
scope: SkillScope::Admin,
},
SkillMetadata {
name: "repo-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
},
];
turn_context.model_info.context_window = Some(100);
turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome));
let initial_context = session.build_initial_context(&turn_context).await;
let developer_texts = developer_input_texts(&initial_context);
assert!(
developer_texts
.iter()
.all(|text| !text.contains("Exceeded skills context budget")),
"expected skill budget warning to stay out of the initial context, got {developer_texts:?}"
);
assert!(
developer_texts
.iter()
.all(|text| !text.contains("- admin-skill:") && !text.contains("- repo-skill:")),
"expected no skill metadata entries to fit the tiny budget, got {developer_texts:?}"
);
}
#[test]
fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() {
let session_telemetry = test_session_telemetry_without_metadata();
let rendered = build_available_skills(
&[SkillMetadata {
name: "repo-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
}],
SkillMetadataBudget::Characters(1),
SkillRenderSideEffects::ThreadStart {
session_telemetry: &session_telemetry,
},
)
.expect("skills should render");
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
.to_string()
)
);
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
assert_eq!(
histogram_sum(&snapshot, THREAD_SKILLS_ENABLED_TOTAL_METRIC),
1
);
assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_KEPT_TOTAL_METRIC), 0);
assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 1);
assert_eq!(
histogram_sum(&snapshot, THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC),
4
);
}
#[test]
fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_omitted_skills() {
let session_telemetry = test_session_telemetry_without_metadata();
let alpha = SkillMetadata {
name: "alpha-skill".to_string(),
description: "abcdef".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/alpha-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
};
let beta = SkillMetadata {
name: "beta-skill".to_string(),
description: "uvwxyz".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/beta-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
};
let minimum_skill_line_cost = |skill: &SkillMetadata| {
let path = skill.path_to_skills_md.to_string_lossy().replace('\\', "/");
format!("- {}: (file: {})\n", skill.name, path)
.chars()
.count()
};
let minimum_budget = minimum_skill_line_cost(&alpha) + minimum_skill_line_cost(&beta);
let rendered = build_available_skills(
&[alpha, beta],
SkillMetadataBudget::Characters(minimum_budget + 6),
SkillRenderSideEffects::ThreadStart {
session_telemetry: &session_telemetry,
},
)
.expect("skills should render");
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(rendered.report.truncated_description_chars, 8);
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 0);
assert_eq!(
histogram_sum(&snapshot, THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC),
8
);
}
#[tokio::test]
async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_builds() {
let (session, turn_context, rx) = make_session_and_context_with_rx().await;
let mut turn_context = Arc::into_inner(turn_context).expect("sole turn context owner");
let mut outcome = SkillLoadOutcome::default();
outcome.skills = vec![
SkillMetadata {
name: "admin-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(),
scope: SkillScope::Admin,
},
SkillMetadata {
name: "repo-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
},
];
turn_context.model_info.context_window = Some(100);
turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome));
let _ = session.build_initial_context(&turn_context).await;
let warning_event = timeout(Duration::from_secs(1), rx.recv())
.await
.expect("warning event should arrive")
.expect("warning event should be readable");
assert!(matches!(
warning_event.msg,
EventMsg::Warning(WarningEvent { message })
if message == "Warning: Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
));
let _ = session.build_initial_context(&turn_context).await;
let warning_event = timeout(Duration::from_secs(1), rx.recv())
.await
.expect("warning event should arrive on repeated build")
.expect("warning event should be readable");
assert!(matches!(
warning_event.msg,
EventMsg::Warning(WarningEvent { message })
if message == "Warning: Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
));
}
#[tokio::test]
async fn handle_output_item_done_records_image_save_history_message() {
let (session, turn_context) = make_session_and_context().await;
@ -7127,7 +6721,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
};
let expected = format!(
"approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
"approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
policy = turn_context.approval_policy.value()
);
@ -7204,7 +6798,7 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
};
let expected = format!(
"approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
"approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
policy = turn_context.approval_policy.value()
);

View file

@ -737,21 +737,14 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
/*model_catalog*/ None,
CollaborationModesConfig::default(),
));
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
/*bundled_skills_enabled*/ true,
));
let mcp_manager = Arc::new(McpManager::new());
let skills_watcher = Arc::new(SkillsWatcher::noop());
let CodexSpawnOk { codex, .. } = Codex::spawn(CodexSpawnArgs {
config,
auth_manager,
models_manager,
environment_manager: Arc::new(EnvironmentManager::default_for_tests()),
skills_manager,
mcp_manager,
skills_watcher,
conversation_history: InitialHistory::New,
session_source: SessionSource::SubAgent(SubAgentSource::Other(
GUARDIAN_REVIEWER_NAME.to_string(),

View file

@ -11,17 +11,6 @@ pub(super) fn image_generation_tool_auth_allowed(auth_manager: Option<&AuthManag
)
}
#[derive(Clone, Debug)]
pub(crate) struct TurnSkillsContext {
pub(crate) outcome: Arc<SkillLoadOutcome>,
}
impl TurnSkillsContext {
pub(crate) fn new(outcome: Arc<SkillLoadOutcome>) -> Self {
Self { outcome }
}
}
#[derive(Clone, Debug)]
pub(crate) struct TurnEnvironment {
#[allow(dead_code)]
@ -76,7 +65,6 @@ pub(crate) struct TurnContext {
pub(crate) js_repl: Arc<JsReplHandle>,
pub(crate) dynamic_tools: Vec<DynamicToolSpec>,
pub(crate) turn_metadata_state: Arc<TurnMetadataState>,
pub(crate) turn_skills: TurnSkillsContext,
pub(crate) turn_timing_state: Arc<TurnTimingState>,
}
impl TurnContext {
@ -204,7 +192,6 @@ impl TurnContext {
js_repl: Arc::clone(&self.js_repl),
dynamic_tools: self.dynamic_tools.clone(),
turn_metadata_state: self.turn_metadata_state.clone(),
turn_skills: self.turn_skills.clone(),
turn_timing_state: Arc::clone(&self.turn_timing_state),
}
}
@ -365,7 +352,6 @@ impl Session {
cwd: AbsolutePathBuf,
sub_id: String,
js_repl: Arc<JsReplHandle>,
skills_outcome: Arc<SkillLoadOutcome>,
) -> TurnContext {
let reasoning_effort = session_configuration.collaboration_mode.reasoning_effort();
let reasoning_summary = session_configuration
@ -457,7 +443,6 @@ impl Session {
js_repl,
dynamic_tools: session_configuration.dynamic_tools.clone(),
turn_metadata_state,
turn_skills: TurnSkillsContext::new(skills_outcome),
turn_timing_state: Arc::new(TurnTimingState::default()),
}
}
@ -614,7 +599,6 @@ impl Session {
&per_turn_config.to_models_manager_config(),
)
.await;
let skills_outcome = Arc::new(SkillLoadOutcome::default());
let mut turn_context: TurnContext = Self::make_turn_context(
self.conversation_id,
Some(Arc::clone(&self.services.auth_manager)),
@ -641,7 +625,6 @@ impl Session {
cwd,
sub_id,
Arc::clone(&self.js_repl),
skills_outcome,
);
turn_context.realtime_active = self.conversation.running_state().await.is_some();

View file

@ -1,25 +0,0 @@
pub use codex_core_skills::SkillDependencyInfo;
pub use codex_core_skills::SkillError;
pub use codex_core_skills::SkillLoadOutcome;
pub use codex_core_skills::SkillMetadata;
pub use codex_core_skills::SkillPolicy;
pub use codex_core_skills::SkillRenderReport;
pub use codex_core_skills::SkillsLoadInput;
pub use codex_core_skills::SkillsManager;
pub use codex_core_skills::build_available_skills;
pub use codex_core_skills::collect_env_var_dependencies;
pub use codex_core_skills::config_rules;
pub use codex_core_skills::default_skill_metadata_budget;
pub use codex_core_skills::detect_implicit_skill_invocation_for_command;
pub use codex_core_skills::filter_skill_load_outcome_for_product;
pub use codex_core_skills::injection;
pub use codex_core_skills::injection::SkillInjections;
pub use codex_core_skills::injection::build_skill_injections;
pub use codex_core_skills::injection::collect_explicit_skill_mentions;
pub use codex_core_skills::loader;
pub use codex_core_skills::manager;
pub use codex_core_skills::model;
pub use codex_core_skills::remote;
pub use codex_core_skills::render;
pub use codex_core_skills::render::SkillRenderSideEffects;
pub use codex_core_skills::system;

View file

@ -1,9 +0,0 @@
//! Compatibility marker for the disabled Codex skill watcher.
pub(crate) struct SkillsWatcher;
impl SkillsWatcher {
pub(crate) fn noop() -> Self {
Self
}
}

View file

@ -2,7 +2,6 @@ use std::collections::HashMap;
use std::sync::Arc;
use crate::RolloutRecorder;
use crate::SkillsManager;
use crate::agent::AgentControl;
use crate::client::ModelClient;
use crate::config::StartedNetworkProxy;
@ -10,7 +9,6 @@ use crate::exec_policy::ExecPolicyManager;
use crate::guardian::GuardianRejection;
use crate::guardian::GuardianRejectionCircuitBreaker;
use crate::mcp::McpManager;
use crate::skills_watcher::SkillsWatcher;
use crate::tools::code_mode::CodeModeService;
use crate::tools::network_approval::NetworkApprovalService;
use crate::tools::sandboxing::ApprovalStore;
@ -55,9 +53,7 @@ pub(crate) struct SessionServices {
pub(crate) guardian_rejections: Mutex<HashMap<String, GuardianRejection>>,
pub(crate) guardian_rejection_circuit_breaker: Mutex<GuardianRejectionCircuitBreaker>,
pub(crate) runtime_handle: Handle,
pub(crate) skills_manager: Arc<SkillsManager>,
pub(crate) mcp_manager: Arc<McpManager>,
pub(crate) skills_watcher: Arc<SkillsWatcher>,
pub(crate) agent_control: AgentControl,
pub(crate) network_proxy: Option<StartedNetworkProxy>,
pub(crate) network_approval: Arc<NetworkApprovalService>,

View file

@ -1,4 +1,3 @@
use crate::SkillsManager;
use crate::agent::AgentControl;
use crate::codex_thread::CodexThread;
use crate::config::Config;
@ -10,7 +9,6 @@ use crate::session::CodexSpawnArgs;
use crate::session::CodexSpawnOk;
use crate::session::INITIAL_SUBMIT_ID;
use crate::shell_snapshot::ShellSnapshot;
use crate::skills_watcher::SkillsWatcher;
use crate::tasks::interrupted_turn_history_marker;
use codex_analytics::AnalyticsEventsClient;
use codex_app_server_protocol::ThreadHistoryBuilder;
@ -42,7 +40,6 @@ use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnAbortedEvent;
use codex_protocol::protocol::W3cTraceContext;
use codex_state::DirectionalThreadSpawnEdgeStatus;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::StreamExt;
use futures::stream::FuturesUnordered;
use std::collections::HashMap;
@ -85,10 +82,6 @@ impl Drop for TempCodexHomeGuard {
}
}
fn build_skills_watcher(_skills_manager: Arc<SkillsManager>) -> Arc<SkillsWatcher> {
Arc::new(SkillsWatcher::noop())
}
/// Represents a newly created Codex thread (formerly called a conversation), including the first event
/// (which is [`EventMsg::SessionConfigured`]).
pub struct NewThread {
@ -165,9 +158,7 @@ pub(crate) struct ThreadManagerState {
auth_manager: Arc<AuthManager>,
models_manager: Arc<ModelsManager>,
environment_manager: Arc<EnvironmentManager>,
skills_manager: Arc<SkillsManager>,
mcp_manager: Arc<McpManager>,
skills_watcher: Arc<SkillsWatcher>,
session_source: SessionSource,
analytics_events_client: Option<AnalyticsEventsClient>,
// Captures submitted ops for testing purpose when test mode is enabled.
@ -203,16 +194,8 @@ impl ThreadManager {
environment_manager: Arc<EnvironmentManager>,
analytics_events_client: Option<AnalyticsEventsClient>,
) -> Self {
let codex_home = config.codex_home.clone();
let restriction_product = session_source.restriction_product();
let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY);
let mcp_manager = Arc::new(McpManager::new());
let skills_manager = Arc::new(SkillsManager::new_with_restriction_product(
codex_home,
config.bundled_skills_enabled(),
restriction_product,
));
let skills_watcher = build_skills_watcher(Arc::clone(&skills_manager));
Self {
state: Arc::new(ThreadManagerState {
threads: Arc::new(RwLock::new(HashMap::new())),
@ -223,9 +206,7 @@ impl ThreadManager {
collaboration_modes_config,
),
environment_manager,
skills_manager,
mcp_manager,
skills_watcher,
auth_manager,
session_source,
analytics_events_client,
@ -269,19 +250,8 @@ impl ThreadManager {
) -> Self {
set_thread_manager_test_mode_for_tests(/*enabled*/ true);
let auth_manager = AuthManager::from_auth_for_testing(auth);
let skills_codex_home = match AbsolutePathBuf::from_absolute_path_checked(&codex_home) {
Ok(codex_home) => codex_home,
Err(err) => panic!("test codex_home should be absolute: {err}"),
};
let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY);
let restriction_product = SessionSource::Exec.restriction_product();
let mcp_manager = Arc::new(McpManager::new());
let skills_manager = Arc::new(SkillsManager::new_with_restriction_product(
skills_codex_home,
/*bundled_skills_enabled*/ true,
restriction_product,
));
let skills_watcher = build_skills_watcher(Arc::clone(&skills_manager));
Self {
state: Arc::new(ThreadManagerState {
threads: Arc::new(RwLock::new(HashMap::new())),
@ -292,9 +262,7 @@ impl ThreadManager {
provider,
)),
environment_manager,
skills_manager,
mcp_manager,
skills_watcher,
auth_manager,
session_source: SessionSource::Exec,
analytics_events_client: None,
@ -313,10 +281,6 @@ impl ThreadManager {
self.state.auth_manager.clone()
}
pub fn skills_manager(&self) -> Arc<SkillsManager> {
self.state.skills_manager.clone()
}
pub fn mcp_manager(&self) -> Arc<McpManager> {
self.state.mcp_manager.clone()
}
@ -871,9 +835,7 @@ impl ThreadManagerState {
auth_manager,
models_manager: Arc::clone(&self.models_manager),
environment_manager: Arc::clone(&self.environment_manager),
skills_manager: Arc::clone(&self.skills_manager),
mcp_manager: Arc::clone(&self.mcp_manager),
skills_watcher: Arc::clone(&self.skills_watcher),
conversation_history: initial_history,
session_source,
agent_control,

View file

@ -1210,7 +1210,6 @@ mod tests {
use crate::test_support::PathBufExt;
use crate::test_support::test_path_buf;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::SkillScope;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyEventKind;