mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Move explicit skill invocation into SkillsExtension
This commit is contained in:
Generated
+2
-1
@@ -2793,7 +2793,6 @@ name = "codex-core-skills"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"codex-analytics",
|
||||
"codex-app-server-protocol",
|
||||
"codex-config",
|
||||
"codex-context-fragments",
|
||||
@@ -3885,6 +3884,8 @@ dependencies = [
|
||||
name = "codex-skills-extension"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"codex-analytics",
|
||||
"codex-connectors",
|
||||
"codex-core-skills",
|
||||
"codex-exec-server",
|
||||
"codex-extension-api",
|
||||
|
||||
@@ -23,13 +23,20 @@ const LOCAL_SKILL_MARKER: &str = "LOCAL_SKILL_BODY_MARKER";
|
||||
#[tokio::test]
|
||||
async fn selected_executor_root_exposes_plugin_skill() -> Result<()> {
|
||||
let server = responses::start_mock_server().await;
|
||||
let response_mock = responses::mount_sse_once(
|
||||
let response_mock = responses::mount_sse_sequence(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-selected"),
|
||||
responses::ev_assistant_message("msg-selected", "Done"),
|
||||
responses::ev_completed("resp-selected"),
|
||||
]),
|
||||
vec![
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-selected"),
|
||||
responses::ev_assistant_message("msg-selected", "Done"),
|
||||
responses::ev_completed("resp-selected"),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-host"),
|
||||
responses::ev_assistant_message("msg-host", "Done"),
|
||||
responses::ev_completed("resp-host"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -145,5 +152,55 @@ stream_max_retries = 0
|
||||
assert!(skill_fragment.contains(SKILL_MARKER));
|
||||
assert!(!skill_fragment.contains(LOCAL_SKILL_MARKER));
|
||||
|
||||
let request_id = app_server
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
READ_TIMEOUT,
|
||||
app_server.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response(response)?;
|
||||
let request_id = app_server
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
input: vec![UserInput::Text {
|
||||
text: format!("Use ${SKILL_NAME}"),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
READ_TIMEOUT,
|
||||
app_server.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
READ_TIMEOUT,
|
||||
app_server.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let host_request = response_mock
|
||||
.requests()
|
||||
.last()
|
||||
.cloned()
|
||||
.expect("host skill request should be captured");
|
||||
let host_skill_fragments = host_request
|
||||
.message_input_texts("user")
|
||||
.into_iter()
|
||||
.filter(|text| text.starts_with("<skill>"))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(1, host_skill_fragments.len());
|
||||
let host_skill_fragment = host_skill_fragments
|
||||
.first()
|
||||
.expect("host skill instructions should be model-visible");
|
||||
assert!(host_skill_fragment.contains(LOCAL_SKILL_MARKER));
|
||||
assert!(!host_skill_fragment.contains(SKILL_MARKER));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerTransportConfig;
|
||||
use tracing::warn;
|
||||
|
||||
/// One MCP server requested by a capability selected for the current turn.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct McpServerDependency {
|
||||
pub source_name: String,
|
||||
pub name: String,
|
||||
pub transport: Option<String>,
|
||||
pub command: Option<String>,
|
||||
pub url: Option<String>,
|
||||
}
|
||||
|
||||
/// Turn-scoped MCP dependencies contributed by capability owners.
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq)]
|
||||
pub struct McpServerDependencies {
|
||||
dependencies: Vec<McpServerDependency>,
|
||||
}
|
||||
|
||||
impl McpServerDependencies {
|
||||
pub fn push(&mut self, dependency: McpServerDependency) {
|
||||
self.dependencies.push(dependency);
|
||||
}
|
||||
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.dependencies.is_empty()
|
||||
}
|
||||
|
||||
pub fn missing_from(
|
||||
&self,
|
||||
installed: &HashMap<String, McpServerConfig>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
let mut missing = HashMap::new();
|
||||
let installed_keys = installed
|
||||
.iter()
|
||||
.map(|(name, config)| canonical_server_key(name, config))
|
||||
.collect::<HashSet<_>>();
|
||||
let mut seen_keys = HashSet::new();
|
||||
|
||||
for dependency in &self.dependencies {
|
||||
let dependency_key = match canonical_dependency_key(dependency) {
|
||||
Ok(key) => key,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
"unable to auto-install MCP dependency {} for {}: {err}",
|
||||
dependency.name, dependency.source_name
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if installed_keys.contains(&dependency_key) || !seen_keys.insert(dependency_key.clone())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
match dependency_to_server_config(dependency) {
|
||||
Ok(config) => {
|
||||
missing.insert(dependency.name.clone(), config);
|
||||
}
|
||||
Err(err) => warn!(
|
||||
"unable to auto-install MCP dependency {dependency_key} for {}: {err}",
|
||||
dependency.source_name
|
||||
),
|
||||
}
|
||||
}
|
||||
missing
|
||||
}
|
||||
}
|
||||
|
||||
pub fn canonical_server_key(name: &str, config: &McpServerConfig) -> String {
|
||||
match &config.transport {
|
||||
McpServerTransportConfig::Stdio { command, .. } => canonical_key("stdio", command, name),
|
||||
McpServerTransportConfig::StreamableHttp { url, .. } => {
|
||||
canonical_key("streamable_http", url, name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn canonical_dependency_key(dependency: &McpServerDependency) -> Result<String, String> {
|
||||
let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
|
||||
if transport.eq_ignore_ascii_case("streamable_http") {
|
||||
let url = dependency
|
||||
.url
|
||||
.as_ref()
|
||||
.ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
|
||||
return Ok(canonical_key("streamable_http", url, &dependency.name));
|
||||
}
|
||||
if transport.eq_ignore_ascii_case("stdio") {
|
||||
let command = dependency
|
||||
.command
|
||||
.as_ref()
|
||||
.ok_or_else(|| "missing command for stdio dependency".to_string())?;
|
||||
return Ok(canonical_key("stdio", command, &dependency.name));
|
||||
}
|
||||
Err(format!("unsupported transport {transport}"))
|
||||
}
|
||||
|
||||
fn dependency_to_server_config(
|
||||
dependency: &McpServerDependency,
|
||||
) -> Result<McpServerConfig, String> {
|
||||
let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
|
||||
let transport = if transport.eq_ignore_ascii_case("streamable_http") {
|
||||
McpServerTransportConfig::StreamableHttp {
|
||||
url: dependency
|
||||
.url
|
||||
.clone()
|
||||
.ok_or_else(|| "missing url for streamable_http dependency".to_string())?,
|
||||
bearer_token_env_var: None,
|
||||
http_headers: None,
|
||||
env_http_headers: None,
|
||||
}
|
||||
} else if transport.eq_ignore_ascii_case("stdio") {
|
||||
McpServerTransportConfig::Stdio {
|
||||
command: dependency
|
||||
.command
|
||||
.clone()
|
||||
.ok_or_else(|| "missing command for stdio dependency".to_string())?,
|
||||
args: Vec::new(),
|
||||
env: None,
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
}
|
||||
} else {
|
||||
return Err(format!("unsupported transport {transport}"));
|
||||
};
|
||||
|
||||
Ok(McpServerConfig {
|
||||
transport,
|
||||
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
oauth: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
})
|
||||
}
|
||||
|
||||
fn canonical_key(transport: &str, identifier: &str, fallback: &str) -> String {
|
||||
let identifier = identifier.trim();
|
||||
if identifier.is_empty() {
|
||||
fallback.to_string()
|
||||
} else {
|
||||
format!("mcp__{transport}__{identifier}")
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "dependencies_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,31 @@
|
||||
use std::collections::HashMap;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::McpServerDependencies;
|
||||
use super::McpServerDependency;
|
||||
use super::canonical_server_key;
|
||||
|
||||
#[test]
|
||||
fn missing_from_deduplicates_by_transport_identity() {
|
||||
let dependency = McpServerDependency {
|
||||
source_name: "demo".to_string(),
|
||||
name: "docs".to_string(),
|
||||
transport: Some("streamable_http".to_string()),
|
||||
command: None,
|
||||
url: Some("https://example.com/mcp".to_string()),
|
||||
};
|
||||
let mut dependencies = McpServerDependencies::default();
|
||||
dependencies.push(dependency.clone());
|
||||
dependencies.push(dependency);
|
||||
|
||||
let missing = dependencies.missing_from(&HashMap::new());
|
||||
|
||||
assert_eq!(missing.len(), 1);
|
||||
assert_eq!(
|
||||
missing
|
||||
.get("docs")
|
||||
.map(|config| canonical_server_key("docs", config)),
|
||||
Some("mcp__streamable_http__https://example.com/mcp".to_string())
|
||||
);
|
||||
}
|
||||
@@ -1,5 +1,8 @@
|
||||
pub use connection_manager::McpConnectionManager;
|
||||
pub use connection_manager::tool_is_model_visible;
|
||||
pub use dependencies::McpServerDependencies;
|
||||
pub use dependencies::McpServerDependency;
|
||||
pub use dependencies::canonical_server_key;
|
||||
pub use elicitation::ElicitationReviewRequest;
|
||||
pub use elicitation::ElicitationReviewer;
|
||||
pub use elicitation::ElicitationReviewerHandle;
|
||||
@@ -74,6 +77,7 @@ pub(crate) mod auth_elicitation;
|
||||
mod catalog;
|
||||
pub(crate) mod codex_apps;
|
||||
pub(crate) mod connection_manager;
|
||||
mod dependencies;
|
||||
pub(crate) mod elicitation;
|
||||
pub(crate) mod mcp;
|
||||
mod plugin_config;
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
|
||||
use crate::metadata::connector_mention_slug;
|
||||
|
||||
/// Connector references discovered by a capability owner while preparing turn input.
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq)]
|
||||
pub struct ExplicitConnectorMentions {
|
||||
connector_ids: HashSet<String>,
|
||||
plain_names: HashSet<String>,
|
||||
}
|
||||
|
||||
impl ExplicitConnectorMentions {
|
||||
pub fn insert_connector_id(&mut self, connector_id: impl Into<String>) {
|
||||
self.connector_ids.insert(connector_id.into());
|
||||
}
|
||||
|
||||
pub fn insert_plain_name(&mut self, name: impl Into<String>) {
|
||||
self.plain_names.insert(name.into().to_ascii_lowercase());
|
||||
}
|
||||
|
||||
pub fn extend(&mut self, other: Self) {
|
||||
self.connector_ids.extend(other.connector_ids);
|
||||
self.plain_names.extend(other.plain_names);
|
||||
}
|
||||
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.connector_ids.is_empty() && self.plain_names.is_empty()
|
||||
}
|
||||
|
||||
pub fn resolve(&self, connectors: &[AppInfo]) -> HashSet<String> {
|
||||
let slug_counts = connectors
|
||||
.iter()
|
||||
.fold(HashMap::new(), |mut counts, connector| {
|
||||
*counts
|
||||
.entry(connector_mention_slug(connector))
|
||||
.or_insert(0usize) += 1;
|
||||
counts
|
||||
});
|
||||
let mut connector_ids = self.connector_ids.clone();
|
||||
connector_ids.extend(connectors.iter().filter_map(|connector| {
|
||||
let slug = connector_mention_slug(connector);
|
||||
(slug_counts.get(&slug) == Some(&1) && self.plain_names.contains(&slug))
|
||||
.then(|| connector.id.clone())
|
||||
}));
|
||||
connector_ids
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "explicit_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,49 @@
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::ExplicitConnectorMentions;
|
||||
|
||||
#[test]
|
||||
fn resolve_keeps_ids_and_unambiguous_plain_names() {
|
||||
let connectors = vec![connector("calendar-1", "Google Calendar")];
|
||||
let mut mentions = ExplicitConnectorMentions::default();
|
||||
mentions.insert_connector_id("direct");
|
||||
mentions.insert_plain_name("google-calendar");
|
||||
|
||||
assert_eq!(
|
||||
mentions.resolve(&connectors),
|
||||
["calendar-1".to_string(), "direct".to_string()]
|
||||
.into_iter()
|
||||
.collect()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_skips_ambiguous_plain_names() {
|
||||
let connectors = vec![
|
||||
connector("calendar-1", "Google Calendar"),
|
||||
connector("calendar-2", "Google Calendar"),
|
||||
];
|
||||
let mut mentions = ExplicitConnectorMentions::default();
|
||||
mentions.insert_plain_name("google-calendar");
|
||||
|
||||
assert_eq!(mentions.resolve(&connectors), Default::default());
|
||||
}
|
||||
|
||||
fn connector(id: &str, name: &str) -> AppInfo {
|
||||
AppInfo {
|
||||
id: id.to_string(),
|
||||
name: name.to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: Vec::new(),
|
||||
}
|
||||
}
|
||||
@@ -14,6 +14,7 @@ use serde::Serialize;
|
||||
pub mod accessible;
|
||||
mod app_tool_policy;
|
||||
mod directory_cache;
|
||||
mod explicit;
|
||||
pub mod filter;
|
||||
pub mod merge;
|
||||
pub mod metadata;
|
||||
@@ -24,6 +25,7 @@ pub use app_tool_policy::AppToolPolicyInput;
|
||||
pub use app_tool_policy::app_is_enabled;
|
||||
pub use app_tool_policy::apps_config_from_layer_stack;
|
||||
pub use directory_cache::ConnectorDirectoryCacheContext;
|
||||
pub use explicit::ExplicitConnectorMentions;
|
||||
|
||||
pub const CONNECTORS_CACHE_TTL: Duration = Duration::from_secs(3600);
|
||||
|
||||
|
||||
@@ -14,7 +14,6 @@ workspace = true
|
||||
|
||||
[dependencies]
|
||||
anyhow = { workspace = true }
|
||||
codex-analytics = { workspace = true }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-config = { workspace = true }
|
||||
codex-context-fragments = { workspace = true }
|
||||
|
||||
@@ -1,135 +1,12 @@
|
||||
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_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL;
|
||||
|
||||
#[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,
|
||||
}
|
||||
|
||||
/// Host skill prompts that have already been injected by an extension for this
|
||||
/// turn.
|
||||
///
|
||||
/// Core uses this to keep the legacy skill-injection path from sending the same
|
||||
/// host `SKILL.md` body again while the skills extension is being wired in.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct InjectedHostSkillPrompts {
|
||||
paths: HashSet<String>,
|
||||
}
|
||||
|
||||
impl InjectedHostSkillPrompts {
|
||||
pub fn insert_path(&mut self, path: impl Into<String>) {
|
||||
let path = path.into();
|
||||
self.paths.insert(normalize_host_skill_path(&path));
|
||||
self.paths.insert(path);
|
||||
}
|
||||
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.paths.is_empty()
|
||||
}
|
||||
|
||||
pub fn contains_path(&self, path: &str) -> bool {
|
||||
self.paths.contains(path) || self.paths.contains(&normalize_host_skill_path(path))
|
||||
}
|
||||
}
|
||||
|
||||
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));
|
||||
let path = PathUri::from_abs_path(&skill.path_to_skills_md);
|
||||
match fs.read_file_text(&path, /*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(),
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
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 normalize_host_skill_path(path: &str) -> String {
|
||||
normalize_skill_path(path).replace('\\', "/")
|
||||
}
|
||||
|
||||
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 structured and text mentions.
|
||||
///
|
||||
/// Structured `UserInput::Skill` selections are resolved first by path against
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
use codex_context_fragments::ContextualUserFragment;
|
||||
|
||||
use crate::injection::SkillInjection;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct SkillInstructions {
|
||||
name: String,
|
||||
@@ -9,16 +7,6 @@ pub struct SkillInstructions {
|
||||
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 {
|
||||
fn role(&self) -> &'static str {
|
||||
"user"
|
||||
|
||||
@@ -75,23 +75,15 @@ pub(crate) mod prompt_debug;
|
||||
#[doc(hidden)]
|
||||
pub use prompt_debug::build_prompt_input;
|
||||
pub(crate) mod mentions {
|
||||
pub(crate) use crate::plugins::build_connector_slug_counts;
|
||||
pub(crate) use crate::plugins::build_skill_name_counts;
|
||||
pub(crate) use crate::plugins::collect_explicit_app_ids;
|
||||
pub(crate) use crate::plugins::collect_explicit_plugin_mentions;
|
||||
pub(crate) use crate::plugins::collect_tool_mentions_from_messages;
|
||||
}
|
||||
mod sandbox_tags;
|
||||
pub mod sandboxing;
|
||||
mod session_prefix;
|
||||
mod session_startup_prewarm;
|
||||
pub mod skills;
|
||||
pub(crate) use skills::SkillInjections;
|
||||
pub(crate) use skills::SkillMetadata;
|
||||
pub(crate) use skills::SkillsService;
|
||||
pub(crate) use skills::build_skill_injections;
|
||||
pub(crate) use skills::build_skill_name_counts;
|
||||
pub(crate) use skills::collect_explicit_skill_mentions;
|
||||
pub(crate) use skills::injection;
|
||||
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
|
||||
pub(crate) use skills::skills_load_input_from_config;
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_config::ConfigEditsBuilder;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerTransportConfig;
|
||||
use codex_config::load_global_mcp_servers;
|
||||
use codex_login::default_client::is_first_party_originator;
|
||||
use codex_login::default_client::originator;
|
||||
@@ -15,13 +13,13 @@ use codex_rmcp_client::perform_oauth_login;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::SkillMetadata;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::skills::model::SkillToolDependency;
|
||||
use codex_mcp::ElicitationReviewerHandle;
|
||||
use codex_mcp::McpOAuthLoginSupport;
|
||||
use codex_mcp::McpPermissionPromptAutoApproveContext;
|
||||
use codex_mcp::McpServerDependencies;
|
||||
use codex_mcp::canonical_server_key;
|
||||
use codex_mcp::mcp_permission_prompt_is_auto_approved;
|
||||
use codex_mcp::oauth_login_support;
|
||||
use codex_mcp::resolve_oauth_scopes;
|
||||
@@ -35,7 +33,7 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
cancellation_token: &CancellationToken,
|
||||
mentioned_skills: &[SkillMetadata],
|
||||
dependencies: &McpServerDependencies,
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) {
|
||||
let originator_value = originator().value;
|
||||
@@ -45,7 +43,7 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
|
||||
}
|
||||
|
||||
let config = turn_context.config.clone();
|
||||
if mentioned_skills.is_empty()
|
||||
if dependencies.is_empty()
|
||||
|| !config
|
||||
.features
|
||||
.enabled(codex_features::Feature::SkillMcpDependencyInstall)
|
||||
@@ -54,7 +52,7 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
|
||||
}
|
||||
|
||||
let installed = sess.runtime_mcp_servers(config.as_ref()).await;
|
||||
let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed);
|
||||
let missing = dependencies.missing_from(&installed);
|
||||
if missing.is_empty() {
|
||||
return;
|
||||
}
|
||||
@@ -71,7 +69,7 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
|
||||
sess,
|
||||
turn_context,
|
||||
config.as_ref(),
|
||||
mentioned_skills,
|
||||
dependencies,
|
||||
elicitation_reviewer,
|
||||
)
|
||||
.await;
|
||||
@@ -82,10 +80,10 @@ pub(crate) async fn maybe_install_mcp_dependencies(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
config: &crate::config::Config,
|
||||
mentioned_skills: &[SkillMetadata],
|
||||
dependencies: &McpServerDependencies,
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) {
|
||||
if mentioned_skills.is_empty()
|
||||
if dependencies.is_empty()
|
||||
|| !config
|
||||
.features
|
||||
.enabled(codex_features::Feature::SkillMcpDependencyInstall)
|
||||
@@ -95,7 +93,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
|
||||
|
||||
let codex_home = config.codex_home.clone();
|
||||
let installed = sess.runtime_mcp_servers(config).await;
|
||||
let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed);
|
||||
let missing = dependencies.missing_from(&installed);
|
||||
if missing.is_empty() {
|
||||
return;
|
||||
}
|
||||
@@ -280,7 +278,7 @@ async fn should_install_mcp_dependencies(
|
||||
|
||||
let prompted_keys = missing
|
||||
.iter()
|
||||
.map(|(name, config)| canonical_mcp_server_key(name, config));
|
||||
.map(|(name, config)| canonical_server_key(name, config));
|
||||
sess.record_mcp_dependency_prompted(prompted_keys).await;
|
||||
|
||||
install
|
||||
@@ -297,7 +295,7 @@ async fn filter_prompted_mcp_dependencies(
|
||||
|
||||
missing
|
||||
.iter()
|
||||
.filter(|(name, config)| !prompted.contains(&canonical_mcp_server_key(name, config)))
|
||||
.filter(|(name, config)| !prompted.contains(&canonical_server_key(name, config)))
|
||||
.map(|(name, config)| (name.clone(), config.clone()))
|
||||
.collect()
|
||||
}
|
||||
@@ -307,165 +305,3 @@ fn format_missing_mcp_dependencies(missing: &HashMap<String, McpServerConfig>) -
|
||||
names.sort();
|
||||
names.join(", ")
|
||||
}
|
||||
|
||||
fn canonical_mcp_key(transport: &str, identifier: &str, fallback: &str) -> String {
|
||||
let identifier = identifier.trim();
|
||||
if identifier.is_empty() {
|
||||
fallback.to_string()
|
||||
} else {
|
||||
format!("mcp__{transport}__{identifier}")
|
||||
}
|
||||
}
|
||||
|
||||
fn canonical_mcp_server_key(name: &str, config: &McpServerConfig) -> String {
|
||||
match &config.transport {
|
||||
McpServerTransportConfig::Stdio { command, .. } => {
|
||||
canonical_mcp_key("stdio", command, name)
|
||||
}
|
||||
McpServerTransportConfig::StreamableHttp { url, .. } => {
|
||||
canonical_mcp_key("streamable_http", url, name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn canonical_mcp_dependency_key(dependency: &SkillToolDependency) -> Result<String, String> {
|
||||
let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
|
||||
if transport.eq_ignore_ascii_case("streamable_http") {
|
||||
let url = dependency
|
||||
.url
|
||||
.as_ref()
|
||||
.ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
|
||||
return Ok(canonical_mcp_key("streamable_http", url, &dependency.value));
|
||||
}
|
||||
if transport.eq_ignore_ascii_case("stdio") {
|
||||
let command = dependency
|
||||
.command
|
||||
.as_ref()
|
||||
.ok_or_else(|| "missing command for stdio dependency".to_string())?;
|
||||
return Ok(canonical_mcp_key("stdio", command, &dependency.value));
|
||||
}
|
||||
Err(format!("unsupported transport {transport}"))
|
||||
}
|
||||
|
||||
fn mcp_dependency_to_server_config(
|
||||
dependency: &SkillToolDependency,
|
||||
) -> Result<McpServerConfig, String> {
|
||||
let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
|
||||
if transport.eq_ignore_ascii_case("streamable_http") {
|
||||
let url = dependency
|
||||
.url
|
||||
.as_ref()
|
||||
.ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
|
||||
return Ok(McpServerConfig {
|
||||
transport: McpServerTransportConfig::StreamableHttp {
|
||||
url: url.clone(),
|
||||
bearer_token_env_var: None,
|
||||
http_headers: None,
|
||||
env_http_headers: None,
|
||||
},
|
||||
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
oauth: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
});
|
||||
}
|
||||
|
||||
if transport.eq_ignore_ascii_case("stdio") {
|
||||
let command = dependency
|
||||
.command
|
||||
.as_ref()
|
||||
.ok_or_else(|| "missing command for stdio dependency".to_string())?;
|
||||
return Ok(McpServerConfig {
|
||||
transport: McpServerTransportConfig::Stdio {
|
||||
command: command.clone(),
|
||||
args: Vec::new(),
|
||||
env: None,
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
oauth: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
});
|
||||
}
|
||||
|
||||
Err(format!("unsupported transport {transport}"))
|
||||
}
|
||||
|
||||
fn collect_missing_mcp_dependencies(
|
||||
mentioned_skills: &[SkillMetadata],
|
||||
installed: &HashMap<String, McpServerConfig>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
let mut missing = HashMap::new();
|
||||
let installed_keys: HashSet<String> = installed
|
||||
.iter()
|
||||
.map(|(name, config)| canonical_mcp_server_key(name, config))
|
||||
.collect();
|
||||
let mut seen_canonical_keys = HashSet::new();
|
||||
|
||||
for skill in mentioned_skills {
|
||||
let Some(dependencies) = skill.dependencies.as_ref() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
for tool in &dependencies.tools {
|
||||
if !tool.r#type.eq_ignore_ascii_case("mcp") {
|
||||
continue;
|
||||
}
|
||||
let dependency_key = match canonical_mcp_dependency_key(tool) {
|
||||
Ok(key) => key,
|
||||
Err(err) => {
|
||||
let dependency = tool.value.as_str();
|
||||
let skill_name = skill.name.as_str();
|
||||
warn!(
|
||||
"unable to auto-install MCP dependency {dependency} for skill {skill_name}: {err}",
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if installed_keys.contains(&dependency_key)
|
||||
|| seen_canonical_keys.contains(&dependency_key)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
let config = match mcp_dependency_to_server_config(tool) {
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
let dependency = dependency_key.as_str();
|
||||
let skill_name = skill.name.as_str();
|
||||
warn!(
|
||||
"unable to auto-install MCP dependency {dependency} for skill {skill_name}: {err}",
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
missing.insert(tool.value.clone(), config);
|
||||
seen_canonical_keys.insert(dependency_key);
|
||||
}
|
||||
}
|
||||
|
||||
missing
|
||||
}
|
||||
|
||||
@@ -1,10 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_connectors::metadata::connector_mention_slug;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
|
||||
use crate::connectors;
|
||||
use crate::injection::ToolMentionKind;
|
||||
use crate::injection::app_id_from_path;
|
||||
use crate::injection::extract_tool_mentions_with_sigil;
|
||||
@@ -15,27 +12,20 @@ use crate::mention_syntax::TOOL_MENTION_SIGIL;
|
||||
|
||||
use super::PluginCapabilitySummary;
|
||||
|
||||
pub(crate) struct CollectedToolMentions {
|
||||
pub(crate) plain_names: HashSet<String>,
|
||||
pub(crate) paths: HashSet<String>,
|
||||
fn collect_tool_paths_from_messages(messages: &[String]) -> HashSet<String> {
|
||||
collect_tool_paths_from_messages_with_sigil(messages, TOOL_MENTION_SIGIL)
|
||||
}
|
||||
|
||||
pub(crate) fn collect_tool_mentions_from_messages(messages: &[String]) -> CollectedToolMentions {
|
||||
collect_tool_mentions_from_messages_with_sigil(messages, TOOL_MENTION_SIGIL)
|
||||
}
|
||||
|
||||
fn collect_tool_mentions_from_messages_with_sigil(
|
||||
fn collect_tool_paths_from_messages_with_sigil(
|
||||
messages: &[String],
|
||||
sigil: char,
|
||||
) -> CollectedToolMentions {
|
||||
let mut plain_names = HashSet::new();
|
||||
) -> HashSet<String> {
|
||||
let mut paths = HashSet::new();
|
||||
for message in messages {
|
||||
let mentions = extract_tool_mentions_with_sigil(message, sigil);
|
||||
plain_names.extend(mentions.plain_names().map(str::to_string));
|
||||
paths.extend(mentions.paths().map(str::to_string));
|
||||
}
|
||||
CollectedToolMentions { plain_names, paths }
|
||||
paths
|
||||
}
|
||||
|
||||
pub(crate) fn collect_explicit_app_ids(input: &[UserInput]) -> HashSet<String> {
|
||||
@@ -53,7 +43,7 @@ pub(crate) fn collect_explicit_app_ids(input: &[UserInput]) -> HashSet<String> {
|
||||
UserInput::Mention { path, .. } => Some(path.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.chain(collect_tool_mentions_from_messages(&messages).paths)
|
||||
.chain(collect_tool_paths_from_messages(&messages))
|
||||
.filter(|path| tool_kind_for_path(path.as_str()) == ToolMentionKind::App)
|
||||
.filter_map(|path| app_id_from_path(path.as_str()).map(str::to_string))
|
||||
.collect()
|
||||
@@ -84,8 +74,7 @@ pub(crate) fn collect_explicit_plugin_mentions(
|
||||
})
|
||||
.chain(
|
||||
// Plugin plaintext links use `@`, not the default `$` tool sigil.
|
||||
collect_tool_mentions_from_messages_with_sigil(&messages, PLUGIN_TEXT_MENTION_SIGIL)
|
||||
.paths,
|
||||
collect_tool_paths_from_messages_with_sigil(&messages, PLUGIN_TEXT_MENTION_SIGIL),
|
||||
)
|
||||
.filter(|path| tool_kind_for_path(path.as_str()) == ToolMentionKind::Plugin)
|
||||
.filter_map(|path| plugin_config_name_from_path(path.as_str()).map(str::to_string))
|
||||
@@ -102,19 +91,6 @@ pub(crate) fn collect_explicit_plugin_mentions(
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) use crate::build_skill_name_counts;
|
||||
|
||||
pub(crate) fn build_connector_slug_counts(
|
||||
connectors: &[connectors::AppInfo],
|
||||
) -> HashMap<String, usize> {
|
||||
let mut counts: HashMap<String, usize> = HashMap::new();
|
||||
for connector in connectors {
|
||||
let slug = connector_mention_slug(connector);
|
||||
*counts.entry(slug).or_insert(0) += 1;
|
||||
}
|
||||
counts
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "mentions_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -11,8 +11,5 @@ pub(crate) use discoverable::list_tool_suggest_discoverable_plugins;
|
||||
pub(crate) use injection::build_plugin_injections;
|
||||
pub(crate) use render::render_explicit_plugin_instructions;
|
||||
|
||||
pub(crate) use mentions::build_connector_slug_counts;
|
||||
pub(crate) use mentions::build_skill_name_counts;
|
||||
pub(crate) use mentions::collect_explicit_app_ids;
|
||||
pub(crate) use mentions::collect_explicit_plugin_mentions;
|
||||
pub(crate) use mentions::collect_tool_mentions_from_messages;
|
||||
|
||||
@@ -227,8 +227,6 @@ use self::session::SessionConfiguration;
|
||||
pub(crate) use self::session::SessionSettingsUpdate;
|
||||
#[cfg(test)]
|
||||
use self::turn::AssistantMessageStreamParsers;
|
||||
#[cfg(test)]
|
||||
use self::turn::collect_explicit_app_ids_from_skill_items;
|
||||
use self::turn::realtime_text_for_event;
|
||||
use self::turn_context::TurnContext;
|
||||
use self::turn_context::TurnSkillsContext;
|
||||
@@ -288,8 +286,6 @@ pub(crate) struct PreviousTurnSettings {
|
||||
pub(crate) realtime_active: Option<bool>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
use crate::SkillMetadata;
|
||||
use crate::SkillsService;
|
||||
use crate::agents_md::load_project_instructions;
|
||||
use crate::exec_policy::ExecPolicyUpdateError;
|
||||
@@ -301,6 +297,8 @@ use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
|
||||
use crate::shell;
|
||||
#[cfg(test)]
|
||||
use crate::skills::SkillLoadOutcome;
|
||||
#[cfg(test)]
|
||||
use crate::skills::SkillMetadata;
|
||||
use crate::state::AutoCompactWindowSnapshot;
|
||||
use crate::state::PendingRequestPermissions;
|
||||
use crate::state::SessionServices;
|
||||
|
||||
@@ -99,6 +99,7 @@ pub(super) async fn spawn_review_thread(
|
||||
let extension_data = Arc::new(codex_extension_api::ExtensionData::new(
|
||||
review_turn_id.clone(),
|
||||
));
|
||||
extension_data.insert(session_telemetry_for_context.clone());
|
||||
extension_data.insert(parent_turn_context.turn_skills.snapshot.clone());
|
||||
|
||||
let review_turn_context = TurnContext {
|
||||
|
||||
@@ -959,6 +959,7 @@ impl Session {
|
||||
));
|
||||
let session_extension_data =
|
||||
codex_extension_api::ExtensionData::new(session_id.to_string());
|
||||
session_extension_data.insert(analytics_events_client.clone());
|
||||
session_extension_data.insert(McpResourceClient::new(Arc::clone(
|
||||
&mcp_connection_manager,
|
||||
)));
|
||||
|
||||
@@ -80,7 +80,6 @@ use crate::tools::handlers::ShellCommandHandler;
|
||||
use crate::tools::registry::ToolExecutor;
|
||||
use crate::tools::router::ToolCallSource;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_app_server_protocol::McpElicitationSchema;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_config::config_toml::ProjectConfig;
|
||||
@@ -257,18 +256,6 @@ fn histogram_sum(resource_metrics: &ResourceMetrics, name: &str) -> u64 {
|
||||
}
|
||||
}
|
||||
|
||||
fn skill_message(text: &str) -> ResponseItem {
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn regular_turn_emits_turn_started_with_trace_id_without_waiting_for_startup_prewarm() {
|
||||
let _trace_test_context = install_test_tracing("codex-core-tests");
|
||||
@@ -584,24 +571,6 @@ fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> T
|
||||
ToolCallRuntime::new(router, session, turn_context, tracker)
|
||||
}
|
||||
|
||||
fn make_connector(id: &str, name: &str) -> AppInfo {
|
||||
AppInfo {
|
||||
id: id.to_string(),
|
||||
name: name.to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assistant_message_stream_parsers_can_be_seeded_from_output_item_added_text() {
|
||||
let mut parsers = AssistantMessageStreamParsers::new(/*plan_mode*/ false);
|
||||
@@ -1538,49 +1507,6 @@ disabled_tools = [
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_explicit_app_ids_from_skill_items_includes_linked_mentions() {
|
||||
let connectors = vec![make_connector("calendar", "Calendar")];
|
||||
let skill_items = vec\n</skill>",
|
||||
)];
|
||||
|
||||
let connector_ids =
|
||||
collect_explicit_app_ids_from_skill_items(&skill_items, &connectors, &HashMap::new());
|
||||
|
||||
assert_eq!(connector_ids, HashSet::from(["calendar".to_string()]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_explicit_app_ids_from_skill_items_resolves_unambiguous_plain_mentions() {
|
||||
let connectors = vec![make_connector("calendar", "Calendar")];
|
||||
let skill_items = vec![skill_message(
|
||||
"<skill>\n<name>demo</name>\n<path>/tmp/skills/demo/SKILL.md</path>\nuse $calendar\n</skill>",
|
||||
)];
|
||||
|
||||
let connector_ids =
|
||||
collect_explicit_app_ids_from_skill_items(&skill_items, &connectors, &HashMap::new());
|
||||
|
||||
assert_eq!(connector_ids, HashSet::from(["calendar".to_string()]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_explicit_app_ids_from_skill_items_skips_plain_mentions_with_skill_conflicts() {
|
||||
let connectors = vec![make_connector("calendar", "Calendar")];
|
||||
let skill_items = vec![skill_message(
|
||||
"<skill>\n<name>demo</name>\n<path>/tmp/skills/demo/SKILL.md</path>\nuse $calendar\n</skill>",
|
||||
)];
|
||||
let skill_name_counts_lower = HashMap::from([("calendar".to_string(), 1)]);
|
||||
|
||||
let connector_ids = collect_explicit_app_ids_from_skill_items(
|
||||
&skill_items,
|
||||
&connectors,
|
||||
&skill_name_counts_lower,
|
||||
);
|
||||
|
||||
assert_eq!(connector_ids, HashSet::<String>::new());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconstruct_history_matches_live_compactions() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -4,12 +4,9 @@ use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use crate::SkillInjections;
|
||||
use crate::build_skill_injections;
|
||||
use crate::client::ModelClientSession;
|
||||
use crate::client_common::Prompt;
|
||||
use crate::client_common::ResponseEvent;
|
||||
use crate::collect_explicit_skill_mentions;
|
||||
use crate::compact::InitialContextInjection;
|
||||
use crate::compact::run_inline_auto_compact_task;
|
||||
use crate::compact::should_use_remote_compact_task;
|
||||
@@ -24,16 +21,10 @@ use crate::hook_runtime::record_pending_input;
|
||||
use crate::hook_runtime::run_legacy_after_agent_hook;
|
||||
use crate::hook_runtime::run_pending_session_start_hooks;
|
||||
use crate::hook_runtime::run_turn_stop_hooks;
|
||||
use crate::injection::ToolMentionKind;
|
||||
use crate::injection::app_id_from_path;
|
||||
use crate::injection::tool_kind_for_path;
|
||||
use crate::mcp_skill_dependencies::maybe_prompt_and_install_mcp_dependencies;
|
||||
use crate::mcp_tool_exposure::build_mcp_tool_exposure;
|
||||
use crate::mentions::build_connector_slug_counts;
|
||||
use crate::mentions::build_skill_name_counts;
|
||||
use crate::mentions::collect_explicit_app_ids;
|
||||
use crate::mentions::collect_explicit_plugin_mentions;
|
||||
use crate::mentions::collect_tool_mentions_from_messages;
|
||||
use crate::plugins::build_plugin_injections;
|
||||
use crate::responses_metadata::CodexResponsesMetadata;
|
||||
use crate::responses_metadata::CodexResponsesRequestKind;
|
||||
@@ -74,7 +65,6 @@ use codex_analytics::TurnResolvedConfigFact;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_async_utils::OrCancelExt;
|
||||
use codex_core_plugins::RecommendedPluginCandidatesInput;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_extension_api::TurnInputContext;
|
||||
use codex_extension_api::TurnInputEnvironment;
|
||||
use codex_features::Feature;
|
||||
@@ -528,57 +518,35 @@ async fn build_skills_and_plugins(
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let skills_outcome = turn_context.turn_skills.snapshot.outcome();
|
||||
let connector_slug_counts = build_connector_slug_counts(&available_connectors);
|
||||
let extension_injection_items =
|
||||
build_extension_turn_input_items(sess, turn_context, &user_input, cancellation_token)
|
||||
.await?;
|
||||
let skill_name_counts_lower =
|
||||
build_skill_name_counts(&skills_outcome.skills, &skills_outcome.disabled_paths).1;
|
||||
let mentioned_skills = collect_explicit_skill_mentions(
|
||||
let reserved_plain_tool_names = available_connectors
|
||||
.iter()
|
||||
.map(codex_connectors::metadata::connector_mention_slug)
|
||||
.collect();
|
||||
let extension_injection_items = build_extension_turn_input_items(
|
||||
sess,
|
||||
turn_context,
|
||||
&user_input,
|
||||
&skills_outcome.skills,
|
||||
&skills_outcome.disabled_paths,
|
||||
&connector_slug_counts,
|
||||
);
|
||||
reserved_plain_tool_names,
|
||||
cancellation_token,
|
||||
)
|
||||
.await?;
|
||||
let mcp_dependencies = turn_context
|
||||
.extension_data
|
||||
.get::<codex_mcp::McpServerDependencies>()
|
||||
.unwrap_or_default();
|
||||
maybe_prompt_and_install_mcp_dependencies(
|
||||
sess,
|
||||
turn_context,
|
||||
cancellation_token,
|
||||
&mentioned_skills,
|
||||
&mcp_dependencies,
|
||||
Some(sess.mcp_elicitation_reviewer()),
|
||||
)
|
||||
.await;
|
||||
|
||||
let injected_host_skill_prompts = turn_context
|
||||
let skill_connector_ids = turn_context
|
||||
.extension_data
|
||||
.get::<InjectedHostSkillPrompts>();
|
||||
let SkillInjections {
|
||||
items: skill_injections,
|
||||
warnings: skill_warnings,
|
||||
} = build_skill_injections(
|
||||
&mentioned_skills,
|
||||
Some(skills_outcome),
|
||||
Some(&turn_context.session_telemetry),
|
||||
&sess.services.analytics_events_client,
|
||||
tracking.clone(),
|
||||
)
|
||||
.await;
|
||||
|
||||
for message in skill_warnings {
|
||||
sess.send_event(turn_context, EventMsg::Warning(WarningEvent { message }))
|
||||
.await;
|
||||
}
|
||||
|
||||
let skill_items: Vec<ResponseItem> = skill_injections
|
||||
.iter()
|
||||
.map(|skill| ContextualUserFragment::into(crate::context::SkillInstructions::from(skill)))
|
||||
.collect();
|
||||
let skill_connector_ids = collect_explicit_app_ids_from_skill_items(
|
||||
&skill_items,
|
||||
&available_connectors,
|
||||
&skill_name_counts_lower,
|
||||
);
|
||||
.get::<codex_connectors::ExplicitConnectorMentions>()
|
||||
.map(|mentions| mentions.resolve(&available_connectors))
|
||||
.unwrap_or_default();
|
||||
let plugin_items =
|
||||
build_plugin_injections(&mentioned_plugins, &mcp_tools, &available_connectors);
|
||||
let mut explicitly_enabled_connectors = collect_explicit_app_ids(&user_input);
|
||||
@@ -609,17 +577,7 @@ async fn build_skills_and_plugins(
|
||||
.track_plugin_used(tracking.clone(), plugin);
|
||||
}
|
||||
|
||||
let mut injection_items: Vec<ResponseItem> = match injected_host_skill_prompts {
|
||||
Some(injected_host_skill_prompts) => skill_injections
|
||||
.iter()
|
||||
.filter(|skill| !injected_host_skill_prompts.contains_path(&skill.path))
|
||||
.map(|skill| {
|
||||
ContextualUserFragment::into(crate::context::SkillInstructions::from(skill))
|
||||
})
|
||||
.collect(),
|
||||
None => skill_items,
|
||||
};
|
||||
injection_items.extend(plugin_items);
|
||||
let mut injection_items = plugin_items;
|
||||
injection_items.extend(extension_injection_items);
|
||||
Some((injection_items, explicitly_enabled_connectors))
|
||||
}
|
||||
@@ -628,6 +586,7 @@ async fn build_extension_turn_input_items(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &TurnContext,
|
||||
user_input: &[UserInput],
|
||||
reserved_plain_tool_names: HashSet<String>,
|
||||
cancellation_token: &CancellationToken,
|
||||
) -> Option<Vec<ResponseItem>> {
|
||||
let contributors = sess.services.extensions.turn_input_contributors().to_vec();
|
||||
@@ -654,7 +613,9 @@ async fn build_extension_turn_input_items(
|
||||
let input = TurnInputContext {
|
||||
thread_id: sess.thread_id(),
|
||||
turn_id: turn_context.sub_id.to_string(),
|
||||
model: turn_context.model_info.slug.clone(),
|
||||
user_input: user_input.to_vec(),
|
||||
reserved_plain_tool_names,
|
||||
environments,
|
||||
};
|
||||
|
||||
@@ -967,57 +928,6 @@ async fn run_auto_compact(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) fn collect_explicit_app_ids_from_skill_items(
|
||||
skill_items: &[ResponseItem],
|
||||
connectors: &[connectors::AppInfo],
|
||||
skill_name_counts_lower: &HashMap<String, usize>,
|
||||
) -> HashSet<String> {
|
||||
if skill_items.is_empty() || connectors.is_empty() {
|
||||
return HashSet::new();
|
||||
}
|
||||
|
||||
let skill_messages = skill_items
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
ResponseItem::Message { content, .. } => {
|
||||
content.iter().find_map(|content_item| match content_item {
|
||||
ContentItem::InputText { text } => Some(text.clone()),
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<String>>();
|
||||
if skill_messages.is_empty() {
|
||||
return HashSet::new();
|
||||
}
|
||||
|
||||
let mentions = collect_tool_mentions_from_messages(&skill_messages);
|
||||
let mention_names_lower = mentions
|
||||
.plain_names
|
||||
.iter()
|
||||
.map(|name| name.to_ascii_lowercase())
|
||||
.collect::<HashSet<String>>();
|
||||
let mut connector_ids = mentions
|
||||
.paths
|
||||
.iter()
|
||||
.filter(|path| tool_kind_for_path(path) == ToolMentionKind::App)
|
||||
.filter_map(|path| app_id_from_path(path).map(str::to_string))
|
||||
.collect::<HashSet<String>>();
|
||||
|
||||
let connector_slug_counts = build_connector_slug_counts(connectors);
|
||||
for connector in connectors {
|
||||
let slug = codex_connectors::metadata::connector_mention_slug(connector);
|
||||
let connector_count = connector_slug_counts.get(&slug).copied().unwrap_or(0);
|
||||
let skill_count = skill_name_counts_lower.get(&slug).copied().unwrap_or(0);
|
||||
if connector_count == 1 && skill_count == 0 && mention_names_lower.contains(&slug) {
|
||||
connector_ids.insert(connector.id.clone());
|
||||
}
|
||||
}
|
||||
|
||||
connector_ids
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub(crate) fn build_prompt(
|
||||
input: Vec<ResponseItem>,
|
||||
|
||||
@@ -22,8 +22,6 @@ 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::model;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::HashSet;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -21,8 +22,15 @@ pub struct TurnInputContext {
|
||||
pub thread_id: ThreadId,
|
||||
/// Stable host-owned turn identifier.
|
||||
pub turn_id: String,
|
||||
/// Model slug selected for this turn.
|
||||
pub model: String,
|
||||
/// User input submitted for this turn.
|
||||
pub user_input: Vec<UserInput>,
|
||||
/// Plain tool names already claimed by host-managed capabilities.
|
||||
///
|
||||
/// Contributors should not resolve these names through ambiguous plaintext
|
||||
/// mentions. Explicit resource or path mentions remain valid.
|
||||
pub reserved_plain_tool_names: HashSet<String>,
|
||||
/// Resolved turn environments, in host priority order.
|
||||
pub environments: Vec<TurnInputEnvironment>,
|
||||
}
|
||||
|
||||
@@ -14,6 +14,8 @@ doctest = false
|
||||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
codex-analytics = { workspace = true }
|
||||
codex-connectors = { workspace = true }
|
||||
codex-core-skills = { workspace = true }
|
||||
codex-exec-server = { workspace = true }
|
||||
codex-extension-api = { workspace = true }
|
||||
|
||||
@@ -1,7 +1,17 @@
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::InvocationType;
|
||||
use codex_analytics::SkillInvocation;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_connectors::ExplicitConnectorMentions;
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_core_skills::SkillMetadata;
|
||||
use codex_core_skills::injection::ToolMentionKind;
|
||||
use codex_core_skills::injection::app_id_from_path;
|
||||
use codex_core_skills::injection::extract_tool_mentions;
|
||||
use codex_core_skills::injection::tool_kind_for_path;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_extension_api::ConfigContributor;
|
||||
use codex_extension_api::ContextContributionContext;
|
||||
@@ -20,6 +30,8 @@ use codex_extension_api::ToolExecutor;
|
||||
use codex_extension_api::TurnInputContext;
|
||||
use codex_extension_api::TurnInputContributor;
|
||||
use codex_mcp::McpResourceClient;
|
||||
use codex_mcp::McpServerDependencies;
|
||||
use codex_mcp::McpServerDependency;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
use codex_protocol::protocol::Event;
|
||||
@@ -112,6 +124,7 @@ where
|
||||
Box::pin(async move {
|
||||
let session_store = context.session_store;
|
||||
let thread_store = context.thread_store;
|
||||
let turn_store = context.turn_store;
|
||||
let Some(thread_state) = thread_store.get::<SkillsThreadState>() else {
|
||||
return Vec::new();
|
||||
};
|
||||
@@ -119,11 +132,11 @@ where
|
||||
if !config.include_instructions {
|
||||
return Vec::new();
|
||||
}
|
||||
let host_snapshot = context.turn_store.get::<HostSkillsSnapshot>();
|
||||
let host_snapshot = turn_store.get::<HostSkillsSnapshot>();
|
||||
let catalog = self
|
||||
.list_skills(
|
||||
SkillListQuery {
|
||||
turn_id: context.turn_store.level_id().to_string(),
|
||||
turn_id: turn_store.level_id().to_string(),
|
||||
executor_roots: thread_state.selected_roots().to_vec(),
|
||||
host_snapshot: host_snapshot.clone(),
|
||||
include_host_skills: true,
|
||||
@@ -206,7 +219,7 @@ where
|
||||
turn_id: input.turn_id.clone(),
|
||||
executor_roots: thread_state.selected_roots().to_vec(),
|
||||
host_snapshot: host_snapshot.clone(),
|
||||
include_host_skills: false,
|
||||
include_host_skills: true,
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
|
||||
mcp_resources: session_store.get::<McpResourceClient>(),
|
||||
@@ -216,11 +229,43 @@ where
|
||||
self.emit_warning(input.thread_id, &input.turn_id, warning.clone());
|
||||
}
|
||||
|
||||
let selected_entries = collect_explicit_skill_mentions(&input.user_input, &catalog);
|
||||
let selected_entries = collect_explicit_skill_mentions(
|
||||
&input.user_input,
|
||||
&catalog,
|
||||
&input.reserved_plain_tool_names,
|
||||
);
|
||||
let mut fragments: Vec<Box<dyn ContextualUserFragment + Send>> = Vec::new();
|
||||
let skill_names = catalog
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.enabled)
|
||||
.map(|entry| entry.name.to_ascii_lowercase())
|
||||
.collect::<HashSet<_>>();
|
||||
let mut mcp_dependencies = McpServerDependencies::default();
|
||||
for entry in &selected_entries {
|
||||
for dependency in entry
|
||||
.dependencies
|
||||
.iter()
|
||||
.flat_map(|dependencies| &dependencies.tools)
|
||||
.filter(|dependency| dependency.r#type.eq_ignore_ascii_case("mcp"))
|
||||
{
|
||||
mcp_dependencies.push(McpServerDependency {
|
||||
source_name: entry.name.clone(),
|
||||
name: dependency.value.clone(),
|
||||
transport: dependency.transport.clone(),
|
||||
command: dependency.command.clone(),
|
||||
url: dependency.url.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
if !mcp_dependencies.is_empty() {
|
||||
turn_store.insert(mcp_dependencies);
|
||||
}
|
||||
|
||||
let mut warnings = catalog.warnings.clone();
|
||||
let mut main_prompts_injected = false;
|
||||
let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default();
|
||||
let mut connector_mentions = ExplicitConnectorMentions::default();
|
||||
let mut skill_invocations = Vec::new();
|
||||
for entry in &selected_entries {
|
||||
match self
|
||||
.read_main_prompt(entry, host_snapshot.clone(), session_store, &thread_state)
|
||||
@@ -229,6 +274,35 @@ where
|
||||
Ok(read_result) => {
|
||||
let (contents, truncated) =
|
||||
truncate_main_prompt_contents(read_result.contents.as_str());
|
||||
let tool_mentions = extract_tool_mentions(&contents);
|
||||
for path in tool_mentions.paths() {
|
||||
if tool_kind_for_path(path) == ToolMentionKind::App
|
||||
&& let Some(connector_id) = app_id_from_path(path)
|
||||
{
|
||||
connector_mentions.insert_connector_id(connector_id);
|
||||
}
|
||||
}
|
||||
for name in tool_mentions.plain_names() {
|
||||
if !skill_names.contains(&name.to_ascii_lowercase()) {
|
||||
connector_mentions.insert_plain_name(name);
|
||||
}
|
||||
}
|
||||
if let Some(skill) = host_skill_for_entry(host_snapshot.as_deref(), entry) {
|
||||
if let Some(telemetry) = turn_store.get::<SessionTelemetry>() {
|
||||
telemetry.counter(
|
||||
"codex.skill.injected",
|
||||
/*inc*/ 1,
|
||||
&[("status", "ok"), ("skill", skill.name.as_str())],
|
||||
);
|
||||
}
|
||||
skill_invocations.push(SkillInvocation {
|
||||
skill_name: skill.name.clone(),
|
||||
skill_scope: skill.scope,
|
||||
skill_path: skill.path_to_skills_md.to_path_buf(),
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
invocation_type: InvocationType::Explicit,
|
||||
});
|
||||
}
|
||||
if truncated {
|
||||
let warning = format!(
|
||||
"Skill `{}` exceeded the main prompt context limit and was truncated.",
|
||||
@@ -248,33 +322,35 @@ where
|
||||
};
|
||||
fragments.push(Box::new(fragment));
|
||||
main_prompts_injected = true;
|
||||
if entry.authority.kind == SkillSourceKind::Host {
|
||||
injected_host_skill_prompts.insert_path(entry.main_prompt.as_str());
|
||||
}
|
||||
}
|
||||
Err(message) => {
|
||||
if let Some(skill) = host_skill_for_entry(host_snapshot.as_deref(), entry)
|
||||
&& let Some(telemetry) = turn_store.get::<SessionTelemetry>()
|
||||
{
|
||||
telemetry.counter(
|
||||
"codex.skill.injected",
|
||||
/*inc*/ 1,
|
||||
&[("status", "error"), ("skill", skill.name.as_str())],
|
||||
);
|
||||
}
|
||||
let warning = format!("Failed to load skill `{}`: {message}", entry.name);
|
||||
self.emit_warning(input.thread_id, &input.turn_id, warning.clone());
|
||||
warnings.push(warning);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(host_snapshot) = &host_snapshot {
|
||||
for entry in selected_entries
|
||||
.iter()
|
||||
.filter(|entry| entry.authority.kind != SkillSourceKind::Host)
|
||||
{
|
||||
for host_skill in host_snapshot
|
||||
.outcome()
|
||||
.skills
|
||||
.iter()
|
||||
.filter(|host_skill| host_skill.name == entry.name)
|
||||
{
|
||||
injected_host_skill_prompts
|
||||
.insert_path(host_skill.path_to_skills_md.to_string_lossy());
|
||||
}
|
||||
}
|
||||
if !connector_mentions.is_empty() {
|
||||
turn_store.insert(connector_mentions);
|
||||
}
|
||||
if let Some(analytics) = session_store.get::<AnalyticsEventsClient>() {
|
||||
analytics.track_skill_invocations(
|
||||
build_track_events_context(
|
||||
input.model,
|
||||
thread_store.level_id().to_string(),
|
||||
input.turn_id,
|
||||
),
|
||||
skill_invocations,
|
||||
);
|
||||
}
|
||||
|
||||
turn_store.insert(SkillsTurnState {
|
||||
@@ -283,9 +359,6 @@ where
|
||||
warnings,
|
||||
main_prompts_injected,
|
||||
});
|
||||
if !injected_host_skill_prompts.is_empty() {
|
||||
turn_store.insert(injected_host_skill_prompts);
|
||||
}
|
||||
|
||||
fragments
|
||||
})
|
||||
@@ -350,6 +423,19 @@ impl<C> SkillsExtension<C> {
|
||||
}
|
||||
}
|
||||
|
||||
fn host_skill_for_entry<'a>(
|
||||
host_snapshot: Option<&'a HostSkillsSnapshot>,
|
||||
entry: &SkillCatalogEntry,
|
||||
) -> Option<&'a SkillMetadata> {
|
||||
if entry.authority.kind != SkillSourceKind::Host {
|
||||
return None;
|
||||
}
|
||||
host_snapshot?.outcome().skills.iter().find(|skill| {
|
||||
let path = skill.path_to_skills_md.to_string_lossy();
|
||||
path == entry.main_prompt.as_str() || path.replace('\\', "/") == entry.main_prompt.as_str()
|
||||
})
|
||||
}
|
||||
|
||||
pub fn install<C>(
|
||||
registry: &mut ExtensionRegistryBuilder<C>,
|
||||
config_from_host: impl Fn(&C) -> SkillsExtensionConfig + Send + Sync + 'static,
|
||||
|
||||
@@ -7,12 +7,14 @@ use crate::catalog::SkillAuthority;
|
||||
use crate::catalog::SkillCatalog;
|
||||
use crate::catalog::SkillCatalogEntry;
|
||||
use crate::catalog::SkillPackageId;
|
||||
use crate::catalog::SkillSourceKind;
|
||||
|
||||
const SKILL_PATH_PREFIX: &str = "skill://";
|
||||
|
||||
pub(crate) fn collect_explicit_skill_mentions(
|
||||
inputs: &[UserInput],
|
||||
catalog: &SkillCatalog,
|
||||
reserved_plain_tool_names: &HashSet<String>,
|
||||
) -> Vec<SkillCatalogEntry> {
|
||||
let mut selected = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
@@ -54,13 +56,29 @@ pub(crate) fn collect_explicit_skill_mentions(
|
||||
if blocked_plain_names.contains(name) {
|
||||
continue;
|
||||
}
|
||||
if let Some(entry) = catalog
|
||||
let matching_entries = catalog
|
||||
.entries
|
||||
.iter()
|
||||
.find(|entry| entry.enabled && entry.name == name)
|
||||
.filter(|entry| entry.enabled && entry.name == name);
|
||||
if let Some(entry) = matching_entries
|
||||
.clone()
|
||||
.find(|entry| entry.authority.kind != SkillSourceKind::Host)
|
||||
{
|
||||
push_selected(entry, &mut seen, &mut selected);
|
||||
continue;
|
||||
}
|
||||
|
||||
let mut host_entries =
|
||||
matching_entries.filter(|entry| entry.authority.kind == SkillSourceKind::Host);
|
||||
let Some(entry) = host_entries.next() else {
|
||||
continue;
|
||||
};
|
||||
if host_entries.next().is_some()
|
||||
|| reserved_plain_tool_names.contains(&name.to_ascii_lowercase())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
push_selected(entry, &mut seen, &mut selected);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,20 +1,25 @@
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_connectors::ExplicitConnectorMentions;
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::SkillLoadOutcome;
|
||||
use codex_core_skills::SkillMetadata;
|
||||
use codex_core_skills::model::SkillDependencies;
|
||||
use codex_core_skills::model::SkillToolDependency;
|
||||
use codex_extension_api::ContextContributionContext;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionEventSink;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_extension_api::ThreadStartInput;
|
||||
use codex_extension_api::TurnInputContext;
|
||||
use codex_mcp::McpServerDependencies;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
@@ -52,9 +57,10 @@ type TestResult = Result<(), Box<dyn std::error::Error>>;
|
||||
static NEXT_CODEX_HOME_ID: AtomicUsize = AtomicUsize::new(0);
|
||||
const DEMO_SKILL_CONTENTS: &str =
|
||||
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse [$calendar](app://calendar).\n";
|
||||
const STATIC_SKILL_CONTENTS: &str = "# Lint Fix\n\nRun the formatter.";
|
||||
|
||||
#[tokio::test]
|
||||
async fn installed_extension_catalog_uses_host_service_snapshot() -> TestResult {
|
||||
async fn installed_extension_uses_host_service_snapshot() -> TestResult {
|
||||
let codex_home = test_codex_home();
|
||||
let skill_path = codex_home.join("skills").join("demo").join("SKILL.md");
|
||||
std::fs::create_dir_all(
|
||||
@@ -90,7 +96,16 @@ async fn installed_extension_catalog_uses_host_service_snapshot() -> TestResult
|
||||
description: "Demo skill.".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
dependencies: Some(SkillDependencies {
|
||||
tools: vec![SkillToolDependency {
|
||||
r#type: "mcp".to_string(),
|
||||
value: "docs".to_string(),
|
||||
description: None,
|
||||
transport: Some("streamable_http".to_string()),
|
||||
command: None,
|
||||
url: Some("https://example.com/mcp".to_string()),
|
||||
}],
|
||||
}),
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
scope: SkillScope::User,
|
||||
@@ -105,11 +120,57 @@ async fn installed_extension_catalog_uses_host_service_snapshot() -> TestResult
|
||||
.contribute(prompt_context(&session_store, &thread_store, &turn_store))
|
||||
.await;
|
||||
|
||||
let fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$demo".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
&thread_store,
|
||||
&turn_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
let expected_catalog = format!(
|
||||
"{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n### How to use skills\n{SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"
|
||||
);
|
||||
let expected_skill = format!(
|
||||
"<skill>\n<name>demo</name>\n<path>{skill_prompt_path}</path>\n{DEMO_SKILL_CONTENTS}\n</skill>"
|
||||
);
|
||||
assert_eq!(1, prompt_fragments.len());
|
||||
assert_eq!(expected_catalog, prompt_fragments[0].text());
|
||||
assert_eq!(
|
||||
vec![("user", expected_skill)],
|
||||
fragments
|
||||
.iter()
|
||||
.map(|fragment| (fragment.role(), fragment.render()))
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
assert_eq!(
|
||||
turn_store
|
||||
.get::<ExplicitConnectorMentions>()
|
||||
.ok_or("expected connector mentions")?
|
||||
.resolve(&[]),
|
||||
["calendar".to_string()].into_iter().collect()
|
||||
);
|
||||
assert_eq!(
|
||||
turn_store
|
||||
.get::<McpServerDependencies>()
|
||||
.ok_or("expected MCP dependencies")?
|
||||
.missing_from(&HashMap::new())
|
||||
.keys()
|
||||
.cloned()
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["docs".to_string()]
|
||||
);
|
||||
|
||||
std::fs::remove_dir_all(codex_home)?;
|
||||
Ok(())
|
||||
@@ -139,6 +200,7 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in
|
||||
entries: executor_entries,
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: STATIC_SKILL_CONTENTS.to_string(),
|
||||
read_requests: Arc::clone(&read_requests),
|
||||
list_calls: None,
|
||||
fail_first_list: false,
|
||||
@@ -154,6 +216,7 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in
|
||||
)],
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: STATIC_SKILL_CONTENTS.to_string(),
|
||||
read_requests: Arc::new(Mutex::new(Vec::new())),
|
||||
list_calls: None,
|
||||
fail_first_list: false,
|
||||
@@ -237,10 +300,12 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$lint-fix please".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
@@ -273,10 +338,12 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-2".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "no skill this time".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
@@ -304,6 +371,7 @@ async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult {
|
||||
)],
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: STATIC_SKILL_CONTENTS.to_string(),
|
||||
read_requests: Arc::new(Mutex::new(Vec::new())),
|
||||
list_calls: Some(Arc::clone(&list_calls)),
|
||||
fail_first_list: true,
|
||||
@@ -350,10 +418,12 @@ async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult {
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: turn_id.to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$first".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
@@ -390,6 +460,7 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: STATIC_SKILL_CONTENTS.to_string(),
|
||||
read_requests: Arc::clone(&read_requests),
|
||||
list_calls: None,
|
||||
fail_first_list: false,
|
||||
@@ -430,10 +501,12 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Mention {
|
||||
name: "lint-fix".to_string(),
|
||||
path: root_b_locator.to_string(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
@@ -463,26 +536,27 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
|
||||
catalog: SkillCatalog {
|
||||
entries: vec![
|
||||
test_entry(
|
||||
SkillSourceKind::Orchestrator,
|
||||
"codex_apps",
|
||||
"orchestrator/visible-skill",
|
||||
"skill://orchestrator/visible-skill/SKILL.md",
|
||||
SkillSourceKind::Host,
|
||||
"host",
|
||||
"host/visible-skill",
|
||||
"visible-skill/SKILL.md",
|
||||
),
|
||||
test_entry(
|
||||
SkillSourceKind::Orchestrator,
|
||||
"codex_apps",
|
||||
"orchestrator/hidden-skill",
|
||||
"skill://orchestrator/hidden-skill/SKILL.md",
|
||||
SkillSourceKind::Host,
|
||||
"host",
|
||||
"host/hidden-skill",
|
||||
"hidden-skill/SKILL.md",
|
||||
)
|
||||
.hidden_from_prompt(),
|
||||
],
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: STATIC_SKILL_CONTENTS.to_string(),
|
||||
read_requests: Arc::clone(&read_requests),
|
||||
list_calls: None,
|
||||
fail_first_list: false,
|
||||
});
|
||||
let providers = SkillProviders::new().with_orchestrator_provider(provider);
|
||||
let providers = SkillProviders::new().with_host_provider(provider);
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
install_with_providers(&mut builder, providers, skills_extension_config);
|
||||
let registry = builder.build();
|
||||
@@ -517,10 +591,12 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$hidden-skill".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
@@ -533,9 +609,9 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
|
||||
assert!(fragments[0].render().contains("<name>hidden-skill</name>"));
|
||||
assert_eq!(
|
||||
vec![(
|
||||
SkillAuthority::new(SkillSourceKind::Orchestrator, "codex_apps"),
|
||||
SkillPackageId("orchestrator/hidden-skill".to_string()),
|
||||
SkillResourceId::new("skill://orchestrator/hidden-skill/SKILL.md"),
|
||||
SkillAuthority::new(SkillSourceKind::Host, "host"),
|
||||
SkillPackageId("host/hidden-skill".to_string()),
|
||||
SkillResourceId::new("hidden-skill/SKILL.md"),
|
||||
)],
|
||||
read_request_keys(&read_requests)
|
||||
);
|
||||
@@ -543,9 +619,207 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_plain_mentions_preserve_legacy_disambiguation() -> TestResult {
|
||||
let read_requests = Arc::new(Mutex::new(Vec::new()));
|
||||
let calendar_locator = "skill://host/calendar/SKILL.md";
|
||||
let provider = Arc::new(StaticSkillProvider {
|
||||
catalog: SkillCatalog {
|
||||
entries: vec![
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId("host/deploy-user".to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Host, "host"),
|
||||
"deploy",
|
||||
"Deploy from the user scope.",
|
||||
SkillResourceId::new("deploy-user/SKILL.md"),
|
||||
),
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId("host/deploy-repo".to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Host, "host"),
|
||||
"deploy",
|
||||
"Deploy from the repo scope.",
|
||||
SkillResourceId::new("deploy-repo/SKILL.md"),
|
||||
),
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId("host/calendar".to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Host, "host"),
|
||||
"calendar",
|
||||
"Use the calendar skill.",
|
||||
SkillResourceId::new("calendar/SKILL.md"),
|
||||
)
|
||||
.with_display_path(calendar_locator),
|
||||
],
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: STATIC_SKILL_CONTENTS.to_string(),
|
||||
read_requests: Arc::clone(&read_requests),
|
||||
list_calls: None,
|
||||
fail_first_list: false,
|
||||
});
|
||||
let providers = SkillProviders::new().with_host_provider(provider);
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
install_with_providers(&mut builder, providers, skills_extension_config);
|
||||
let registry = builder.build();
|
||||
let session_store = ExtensionData::new("session");
|
||||
let thread_store = ExtensionData::new("thread");
|
||||
let session_source = SessionSource::Cli;
|
||||
let config = default_config();
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
session_source: &session_source,
|
||||
persistent_thread_state_available: true,
|
||||
environments: &[],
|
||||
session_store: &session_store,
|
||||
thread_store: &thread_store,
|
||||
})
|
||||
.await;
|
||||
|
||||
let ambiguous_fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-ambiguous".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$deploy".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
&thread_store,
|
||||
&ExtensionData::new("turn-ambiguous"),
|
||||
)
|
||||
.await;
|
||||
assert!(ambiguous_fragments.is_empty());
|
||||
|
||||
let connector_conflict_fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-connector-conflict".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$calendar".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: ["calendar".to_string()].into_iter().collect(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
&thread_store,
|
||||
&ExtensionData::new("turn-connector-conflict"),
|
||||
)
|
||||
.await;
|
||||
assert!(connector_conflict_fragments.is_empty());
|
||||
|
||||
let explicit_fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-explicit".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Mention {
|
||||
name: "calendar".to_string(),
|
||||
path: calendar_locator.to_string(),
|
||||
}],
|
||||
reserved_plain_tool_names: ["calendar".to_string()].into_iter().collect(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
&thread_store,
|
||||
&ExtensionData::new("turn-explicit"),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(1, explicit_fragments.len());
|
||||
assert_eq!(
|
||||
vec![(
|
||||
SkillAuthority::new(SkillSourceKind::Host, "host"),
|
||||
SkillPackageId("host/calendar".to_string()),
|
||||
SkillResourceId::new("calendar/SKILL.md"),
|
||||
)],
|
||||
read_request_keys(&read_requests)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn connector_mentions_only_include_injected_skill_text() -> TestResult {
|
||||
let provider = Arc::new(StaticSkillProvider {
|
||||
catalog: SkillCatalog {
|
||||
entries: vec![test_entry(
|
||||
SkillSourceKind::Executor,
|
||||
"env-1",
|
||||
"executor/lint-fix",
|
||||
"lint-fix/SKILL.md",
|
||||
)],
|
||||
warnings: Vec::new(),
|
||||
},
|
||||
contents: format!("{}[$calendar](app://calendar)", "x".repeat(9_000)),
|
||||
read_requests: Arc::new(Mutex::new(Vec::new())),
|
||||
list_calls: None,
|
||||
fail_first_list: false,
|
||||
});
|
||||
let providers = SkillProviders::new().with_executor_provider(provider);
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
install_with_providers(&mut builder, providers, skills_extension_config);
|
||||
let registry = builder.build();
|
||||
let session_store = ExtensionData::new("session");
|
||||
let thread_store = ExtensionData::new("thread");
|
||||
thread_store.insert(vec![SelectedCapabilityRoot {
|
||||
id: "lint-fix".to_string(),
|
||||
location: CapabilityRootLocation::Environment {
|
||||
environment_id: "env-1".to_string(),
|
||||
path: "/skills/lint-fix".to_string(),
|
||||
},
|
||||
}]);
|
||||
let session_source = SessionSource::Cli;
|
||||
let config = default_config();
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
session_source: &session_source,
|
||||
persistent_thread_state_available: true,
|
||||
environments: &[],
|
||||
session_store: &session_store,
|
||||
thread_store: &thread_store,
|
||||
})
|
||||
.await;
|
||||
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
let fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
TurnInputContext {
|
||||
thread_id: ThreadId::default(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
model: "test-model".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "$lint-fix".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
reserved_plain_tool_names: Default::default(),
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
&thread_store,
|
||||
&turn_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(1, fragments.len());
|
||||
assert!(!fragments[0].render().contains("app://calendar"));
|
||||
assert!(turn_store.get::<ExplicitConnectorMentions>().is_none());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct StaticSkillProvider {
|
||||
catalog: SkillCatalog,
|
||||
contents: String,
|
||||
read_requests: Arc<Mutex<Vec<SkillReadRequest>>>,
|
||||
list_calls: Option<Arc<AtomicUsize>>,
|
||||
fail_first_list: bool,
|
||||
@@ -577,6 +851,7 @@ impl SkillProvider for StaticSkillProvider {
|
||||
}
|
||||
|
||||
fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> {
|
||||
let contents = self.contents.clone();
|
||||
let read_requests = Arc::clone(&self.read_requests);
|
||||
Box::pin(async move {
|
||||
read_requests
|
||||
@@ -585,7 +860,7 @@ impl SkillProvider for StaticSkillProvider {
|
||||
.push(request.clone());
|
||||
Ok(SkillReadResult {
|
||||
resource: request.resource,
|
||||
contents: "# Lint Fix\n\nRun the formatter.".to_string(),
|
||||
contents,
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user