mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Expose executor skills through skill tools (#35184)
## What changed - Add executor authority support to `skills.list` and `skills.read` for skills from selected capability roots. - Allow `skills.read` to load package-relative resources referenced by an executor skill while keeping reads within the selected package. - Paginate skill listings and resource contents with bounded response sizes and stale-cursor validation. - Resolve executor filesystems per sampling step so skill tools use the active selected capability roots. ## Testing - Extend the app-server executor skill test to list an executor skill, read its main `SKILL.md`, and read a paginated reference resource. - Cover visibility filtering, metadata bounds, pagination, invalid cursors, package boundaries, and resource size limits. GitOrigin-RevId: 1b5de95d5d1b9f79c34d2879d6e67f24401e9c29
This commit is contained in:
1 parent
1a817bb95d
commit
7c71783135
22 files changed
+704
-163
No files matched your search
@@ -59,6 +59,19 @@ impl SkillAuthority {
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
|
||||
pub struct SkillPackageId(pub String);
|
||||
|
||||
impl SkillPackageId {
|
||||
pub(crate) fn relative_resource_path<'a>(&self, resource: &'a str) -> Option<&'a str> {
|
||||
let relative = resource
|
||||
.strip_prefix(self.0.trim_end_matches('/'))?
|
||||
.strip_prefix('/')?;
|
||||
(!relative.is_empty()
|
||||
&& relative
|
||||
.split('/')
|
||||
.all(|segment| !matches!(segment, "" | "." | "..")))
|
||||
.then_some(relative)
|
||||
}
|
||||
}
|
||||
|
||||
/// Opaque resource id inside a skill package, optionally bound to the
|
||||
/// environment path that owns its contents.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
|
||||
@@ -80,10 +93,12 @@ impl SkillResourceId {
|
||||
environment_id: impl Into<String>,
|
||||
path: PathUri,
|
||||
) -> Self {
|
||||
let package_root = path.parent().unwrap_or_else(|| path.clone());
|
||||
Self {
|
||||
id: id.into(),
|
||||
environment_path: Some(EnvironmentSkillResource {
|
||||
environment_id: environment_id.into(),
|
||||
package_root,
|
||||
path,
|
||||
contents: None,
|
||||
}),
|
||||
@@ -96,10 +111,12 @@ impl SkillResourceId {
|
||||
path: PathUri,
|
||||
contents: String,
|
||||
) -> Self {
|
||||
let package_root = path.parent().unwrap_or_else(|| path.clone());
|
||||
Self {
|
||||
id: id.into(),
|
||||
environment_path: Some(EnvironmentSkillResource {
|
||||
environment_id: environment_id.into(),
|
||||
package_root,
|
||||
path,
|
||||
contents: Some(contents.into()),
|
||||
}),
|
||||
@@ -110,6 +127,26 @@ impl SkillResourceId {
|
||||
&self.id
|
||||
}
|
||||
|
||||
pub(crate) fn bind_environment_package_resource(
|
||||
&self,
|
||||
package: &SkillPackageId,
|
||||
resource: impl Into<String>,
|
||||
) -> Option<Self> {
|
||||
let resource = resource.into();
|
||||
let relative = package.relative_resource_path(&resource)?;
|
||||
let environment = self.environment_path.as_ref()?;
|
||||
let path = environment.package_root.join(relative).ok()?;
|
||||
path.starts_with(&environment.package_root).then(|| Self {
|
||||
id: resource,
|
||||
environment_path: Some(EnvironmentSkillResource {
|
||||
environment_id: environment.environment_id.clone(),
|
||||
package_root: environment.package_root.clone(),
|
||||
path,
|
||||
contents: None,
|
||||
}),
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn environment_path(&self) -> Option<(&str, &PathUri)> {
|
||||
self.environment_path
|
||||
.as_ref()
|
||||
@@ -126,6 +163,7 @@ impl SkillResourceId {
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
|
||||
struct EnvironmentSkillResource {
|
||||
environment_id: String,
|
||||
package_root: PathUri,
|
||||
path: PathUri,
|
||||
contents: Option<Arc<str>>,
|
||||
}
|
||||
@@ -207,6 +245,10 @@ impl SkillCatalogEntry {
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn is_model_visible(&self) -> bool {
|
||||
self.enabled && self.prompt_visible
|
||||
}
|
||||
|
||||
pub(crate) fn rendered_path(&self) -> &str {
|
||||
self.display_path
|
||||
.as_deref()
|
||||
|
||||
@@ -4,6 +4,7 @@ use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::injection::HostSkillsCatalogInWorldState;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_exec_server::ResolvedSelectedCapabilityRoot;
|
||||
use codex_extension_api::ConfigContributor;
|
||||
use codex_extension_api::ContextContributor;
|
||||
use codex_extension_api::ContextualUserFragment;
|
||||
@@ -157,6 +158,7 @@ where
|
||||
SkillListQuery {
|
||||
turn_id: thread_store.level_id().to_string(),
|
||||
executor_roots: Vec::new(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
host_snapshot: None,
|
||||
include_host_skills: false,
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
@@ -207,6 +209,7 @@ where
|
||||
SkillListQuery {
|
||||
turn_id: input.turn_id.to_string(),
|
||||
executor_roots: input.ready_selected_capability_roots.to_vec(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
host_snapshot: None,
|
||||
include_host_skills: false,
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
@@ -278,23 +281,34 @@ where
|
||||
session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
let Some(thread_state) = thread_store.get::<SkillsThreadState>() else {
|
||||
return Vec::new();
|
||||
};
|
||||
if !self.providers.has_orchestrator_provider()
|
||||
|| !thread_state.orchestrator_skills_enabled()
|
||||
{
|
||||
return Vec::new();
|
||||
}
|
||||
self.build_skill_tools(session_store, thread_store, /*executor_query*/ None)
|
||||
}
|
||||
|
||||
skill_tools(
|
||||
self.providers.clone(),
|
||||
session_store
|
||||
.get::<SkillsSessionState>()
|
||||
.and_then(|state| state.mcp_resources.clone()),
|
||||
thread_state,
|
||||
Arc::clone(&self.shadow_selection),
|
||||
)
|
||||
fn tools_for_step(
|
||||
&self,
|
||||
session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
step_store: &ExtensionData,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
let resolved_executor_roots = step_store
|
||||
.get::<Vec<ResolvedSelectedCapabilityRoot>>()
|
||||
.map(|roots| roots.as_slice().to_vec())
|
||||
.unwrap_or_default();
|
||||
let executor_query = (!resolved_executor_roots.is_empty()).then(|| SkillListQuery {
|
||||
turn_id: step_store.level_id().to_string(),
|
||||
executor_roots: resolved_executor_roots
|
||||
.iter()
|
||||
.map(|root| root.selected_root().clone())
|
||||
.collect(),
|
||||
resolved_executor_roots,
|
||||
host_snapshot: None,
|
||||
include_host_skills: false,
|
||||
include_bundled_skills: false,
|
||||
include_orchestrator_skills: false,
|
||||
mcp_resources: None,
|
||||
executor_capability_discovery: None,
|
||||
});
|
||||
self.build_skill_tools(session_store, thread_store, executor_query)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -350,6 +364,7 @@ where
|
||||
let query = SkillListQuery {
|
||||
turn_id: input.turn_id.clone(),
|
||||
executor_roots: Vec::new(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
host_snapshot: host_snapshot.clone(),
|
||||
include_host_skills: !host_catalog_in_world_state,
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
@@ -501,6 +516,33 @@ where
|
||||
}
|
||||
|
||||
impl<C> SkillsExtension<C> {
|
||||
fn build_skill_tools(
|
||||
&self,
|
||||
session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
executor_query: Option<SkillListQuery>,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
let Some(thread_state) = thread_store.get::<SkillsThreadState>() else {
|
||||
return Vec::new();
|
||||
};
|
||||
let orchestrator_available = self.providers.has_orchestrator_provider()
|
||||
&& thread_state.orchestrator_skills_enabled();
|
||||
if !orchestrator_available && executor_query.is_none() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
skill_tools(
|
||||
self.providers.clone(),
|
||||
session_store
|
||||
.get::<SkillsSessionState>()
|
||||
.and_then(|state| state.mcp_resources.clone()),
|
||||
thread_state,
|
||||
orchestrator_available,
|
||||
executor_query,
|
||||
Arc::clone(&self.shadow_selection),
|
||||
)
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all)]
|
||||
async fn list_skills(
|
||||
&self,
|
||||
@@ -541,6 +583,7 @@ impl<C> SkillsExtension<C> {
|
||||
authority: entry.authority.clone(),
|
||||
package: entry.id.clone(),
|
||||
resource: entry.main_prompt.clone(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
host_snapshot,
|
||||
mcp_resources,
|
||||
},
|
||||
|
||||
@@ -8,6 +8,7 @@ mod orchestrator;
|
||||
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_exec_server::ExecutorCapabilityDiscoverySnapshot;
|
||||
use codex_exec_server::ResolvedSelectedCapabilityRoot;
|
||||
use codex_mcp::McpResourceClient;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
|
||||
@@ -23,10 +24,13 @@ pub use executor::ExecutorSkillProvider;
|
||||
pub use host::HostSkillProvider;
|
||||
pub use orchestrator::OrchestratorSkillProvider;
|
||||
|
||||
pub(crate) const MAX_SKILL_RESOURCE_CONTENT_BYTES: usize = 1024 * 1024;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct SkillListQuery {
|
||||
pub turn_id: String,
|
||||
pub executor_roots: Vec<SelectedCapabilityRoot>,
|
||||
pub resolved_executor_roots: Vec<ResolvedSelectedCapabilityRoot>,
|
||||
pub host_snapshot: Option<Arc<HostSkillsSnapshot>>,
|
||||
pub include_host_skills: bool,
|
||||
pub include_bundled_skills: bool,
|
||||
@@ -41,6 +45,7 @@ pub struct SkillReadRequest {
|
||||
pub authority: SkillAuthority,
|
||||
pub package: SkillPackageId,
|
||||
pub resource: SkillResourceId,
|
||||
pub resolved_executor_roots: Vec<ResolvedSelectedCapabilityRoot>,
|
||||
pub host_snapshot: Option<Arc<HostSkillsSnapshot>>,
|
||||
pub mcp_resources: Option<Arc<McpResourceClient>>,
|
||||
}
|
||||
|
||||
@@ -7,6 +7,8 @@ use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_skills::EnvironmentSkillMetadata;
|
||||
use codex_utils_path_uri::PathConvention;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::StreamExt;
|
||||
|
||||
use crate::catalog::SkillAuthority;
|
||||
use crate::catalog::SkillCatalog;
|
||||
@@ -17,6 +19,7 @@ use crate::catalog::SkillReadResult;
|
||||
use crate::catalog::SkillResourceId;
|
||||
use crate::catalog::SkillSearchResult;
|
||||
use crate::catalog::SkillSourceKind;
|
||||
use crate::provider::MAX_SKILL_RESOURCE_CONTENT_BYTES;
|
||||
use crate::provider::SkillListQuery;
|
||||
use crate::provider::SkillProvider;
|
||||
use crate::provider::SkillProviderFuture;
|
||||
@@ -50,24 +53,32 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
}
|
||||
let mut catalog = SkillCatalog::default();
|
||||
for selected_root in query.executor_roots {
|
||||
let selected_root_id = selected_root.id;
|
||||
let selected_root_id = &selected_root.id;
|
||||
let CapabilityRootLocation::Environment {
|
||||
environment_id,
|
||||
path,
|
||||
} = selected_root.location;
|
||||
} = &selected_root.location;
|
||||
let authority =
|
||||
SkillAuthority::new(SkillSourceKind::Executor, selected_root_id.clone());
|
||||
let Some(environment) = self.environment_manager.get_environment(&environment_id)
|
||||
else {
|
||||
let file_system = query
|
||||
.resolved_executor_roots
|
||||
.iter()
|
||||
.find(|root| root.selected_root() == &selected_root)
|
||||
.map(|root| root.environment().get_filesystem())
|
||||
.or_else(|| {
|
||||
self.environment_manager
|
||||
.get_environment(environment_id)
|
||||
.map(|environment| environment.get_filesystem())
|
||||
});
|
||||
let Some(file_system) = file_system else {
|
||||
catalog.warnings.push(format!(
|
||||
"Selected capability root `{selected_root_id}` references unavailable environment `{environment_id}`."
|
||||
));
|
||||
continue;
|
||||
};
|
||||
let file_system = environment.get_filesystem();
|
||||
let outcome = load_environment_skills_from_root(
|
||||
file_system.as_ref(),
|
||||
&path,
|
||||
path,
|
||||
self.restriction_product,
|
||||
)
|
||||
.await;
|
||||
@@ -76,8 +87,8 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
catalog.push_entry(catalog_entry_from_skill(
|
||||
&skill,
|
||||
authority.clone(),
|
||||
&selected_root_id,
|
||||
&environment_id,
|
||||
selected_root_id,
|
||||
environment_id,
|
||||
/*instructions*/ None,
|
||||
));
|
||||
}
|
||||
@@ -95,7 +106,13 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
request.authority.kind
|
||||
)));
|
||||
}
|
||||
if request.package.0 != request.resource.as_str() {
|
||||
let expected_package_prefix = format!("skill://{}/", request.authority.id);
|
||||
if !request.package.0.starts_with(&expected_package_prefix)
|
||||
|| request
|
||||
.package
|
||||
.relative_resource_path(request.resource.as_str())
|
||||
.is_none()
|
||||
{
|
||||
return Err(SkillProviderError::new(
|
||||
"executor skill resource does not match its package",
|
||||
));
|
||||
@@ -111,21 +128,27 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
"executor skill resource is not bound to an environment",
|
||||
));
|
||||
};
|
||||
let Some(environment) = self.environment_manager.get_environment(environment_id) else {
|
||||
let file_system = request
|
||||
.resolved_executor_roots
|
||||
.iter()
|
||||
.find(|root| root.selected_root().id == request.authority.id)
|
||||
.map(|root| root.environment().get_filesystem())
|
||||
.or_else(|| {
|
||||
self.environment_manager
|
||||
.get_environment(environment_id)
|
||||
.map(|environment| environment.get_filesystem())
|
||||
});
|
||||
let Some(file_system) = file_system else {
|
||||
return Err(SkillProviderError::new(format!(
|
||||
"executor skill resource references unavailable environment `{environment_id}`"
|
||||
)));
|
||||
};
|
||||
let contents = environment
|
||||
.get_filesystem()
|
||||
.read_file_text(resource_path, /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
SkillProviderError::new(format!(
|
||||
"failed to read executor skill resource {}: {err}",
|
||||
request.resource.as_str()
|
||||
))
|
||||
})?;
|
||||
let contents = read_bounded_text(
|
||||
file_system.as_ref(),
|
||||
resource_path,
|
||||
request.resource.as_str(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok(SkillReadResult {
|
||||
resource: request.resource,
|
||||
@@ -184,37 +207,42 @@ fn catalog_entry_from_skill(
|
||||
environment_id: &str,
|
||||
instructions: Option<String>,
|
||||
) -> SkillCatalogEntry {
|
||||
let skill_path = skill.path_to_skills_md.inferred_native_path_string();
|
||||
let normalized_path = match skill.path_to_skills_md.infer_path_convention() {
|
||||
Some(PathConvention::Windows) => skill_path.replace('\\', "/"),
|
||||
Some(PathConvention::Posix) | None => skill_path,
|
||||
};
|
||||
let display_path = format!(
|
||||
"skill://{selected_root_id}/{}",
|
||||
normalized_path.trim_start_matches('/')
|
||||
let handle_prefix = format!("skill://{selected_root_id}/");
|
||||
let normalized_main_path = normalized_environment_path(&skill.path_to_skills_md);
|
||||
let normalized_package_path = skill.path_to_skills_md.parent().map_or_else(
|
||||
|| normalized_main_path.clone(),
|
||||
|path| normalized_environment_path(&path),
|
||||
);
|
||||
let package = format!(
|
||||
"{handle_prefix}{}",
|
||||
normalized_package_path.trim_start_matches('/')
|
||||
);
|
||||
let main_resource = format!(
|
||||
"{handle_prefix}{}",
|
||||
normalized_main_path.trim_start_matches('/')
|
||||
);
|
||||
let main_prompt = match instructions {
|
||||
Some(contents) => SkillResourceId::environment_with_contents(
|
||||
display_path.clone(),
|
||||
main_resource.clone(),
|
||||
environment_id,
|
||||
skill.path_to_skills_md.clone(),
|
||||
contents,
|
||||
),
|
||||
None => SkillResourceId::environment(
|
||||
display_path.clone(),
|
||||
main_resource.clone(),
|
||||
environment_id,
|
||||
skill.path_to_skills_md.clone(),
|
||||
),
|
||||
};
|
||||
let entry = SkillCatalogEntry::new(
|
||||
SkillPackageId(display_path.clone()),
|
||||
SkillPackageId(package),
|
||||
authority,
|
||||
skill.name.clone(),
|
||||
skill.description.clone(),
|
||||
main_prompt,
|
||||
)
|
||||
.with_short_description(skill.short_description.clone())
|
||||
.with_display_path(display_path)
|
||||
.with_display_path(main_resource)
|
||||
.with_dependencies(skill.dependencies.clone());
|
||||
|
||||
if skill.allows_implicit_invocation() {
|
||||
@@ -223,3 +251,43 @@ fn catalog_entry_from_skill(
|
||||
entry.hidden_from_prompt()
|
||||
}
|
||||
}
|
||||
|
||||
fn normalized_environment_path(path: &PathUri) -> String {
|
||||
let convention = path.infer_path_convention();
|
||||
let path = path.inferred_native_path_string();
|
||||
match convention {
|
||||
Some(PathConvention::Windows) => path.replace('\\', "/"),
|
||||
Some(PathConvention::Posix) | None => path,
|
||||
}
|
||||
}
|
||||
|
||||
async fn read_bounded_text(
|
||||
file_system: &dyn codex_exec_server::ExecutorFileSystem,
|
||||
path: &PathUri,
|
||||
resource: &str,
|
||||
) -> Result<String, SkillProviderError> {
|
||||
let read_error = |err| {
|
||||
SkillProviderError::new(format!(
|
||||
"failed to read executor skill resource {resource}: {err}"
|
||||
))
|
||||
};
|
||||
let mut stream = file_system
|
||||
.read_file_stream(path, /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(&read_error)?;
|
||||
let mut contents = Vec::new();
|
||||
while let Some(chunk) = stream.next().await {
|
||||
let chunk = chunk.map_err(&read_error)?;
|
||||
if contents.len().saturating_add(chunk.len()) > MAX_SKILL_RESOURCE_CONTENT_BYTES {
|
||||
return Err(SkillProviderError::new(format!(
|
||||
"executor skill resource {resource} exceeds {MAX_SKILL_RESOURCE_CONTENT_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
contents.extend_from_slice(&chunk);
|
||||
}
|
||||
String::from_utf8(contents).map_err(|_| {
|
||||
SkillProviderError::new(format!(
|
||||
"executor skill resource {resource} is not valid UTF-8"
|
||||
))
|
||||
})
|
||||
}
|
||||
@@ -15,6 +15,7 @@ use crate::catalog::SkillReadResult;
|
||||
use crate::catalog::SkillResourceId;
|
||||
use crate::catalog::SkillSearchResult;
|
||||
use crate::catalog::SkillSourceKind;
|
||||
use crate::provider::MAX_SKILL_RESOURCE_CONTENT_BYTES;
|
||||
use crate::provider::SkillListQuery;
|
||||
use crate::provider::SkillProvider;
|
||||
use crate::provider::SkillProviderFuture;
|
||||
@@ -30,8 +31,6 @@ const MAX_SKILL_NAME_CHARS: usize = 64;
|
||||
const MAX_QUALIFIED_SKILL_NAME_CHARS: usize = 128;
|
||||
const MAX_SKILL_PACKAGE_URI_CHARS: usize = 1_024;
|
||||
const MAX_SKILL_RESOURCE_URI_CHARS: usize = 2_048;
|
||||
const MAX_SKILL_RESOURCE_CONTENT_BYTES: usize = 1024 * 1024;
|
||||
|
||||
/// Discovers and reads skills owned by the orchestrator.
|
||||
///
|
||||
/// The provider uses session-scoped resources without exposing the transport or
|
||||
|
||||
@@ -483,7 +483,7 @@ pub(crate) fn render_available_skills(
|
||||
let mut entries = catalog
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.enabled && entry.prompt_visible)
|
||||
.filter(|entry| entry.is_model_visible())
|
||||
.collect::<Vec<_>>();
|
||||
policy.order_entries(&mut entries);
|
||||
if entries.is_empty() {
|
||||
|
||||
@@ -64,8 +64,7 @@ impl ShadowSelectionExperiment {
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(_, entry)| {
|
||||
entry.enabled
|
||||
&& entry.prompt_visible
|
||||
entry.is_model_visible()
|
||||
// Invocation observation currently exists only for host shell use and
|
||||
// orchestrator reads. Keep the candidate set aligned with that universe.
|
||||
&& matches!(
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use codex_extension_api::FunctionCallError;
|
||||
use codex_extension_api::ToolCall;
|
||||
use codex_extension_api::ToolExecutor;
|
||||
use codex_extension_api::ToolExecutorFuture;
|
||||
@@ -8,27 +9,38 @@ use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::catalog::SkillCatalogEntry;
|
||||
use crate::render::MAX_SKILL_NAME_BYTES;
|
||||
use crate::render::truncate_catalog_skill_description;
|
||||
use crate::render::truncate_utf8_to_bytes;
|
||||
use crate::warnings::bounded_warnings;
|
||||
|
||||
use super::MAX_HANDLE_BYTES;
|
||||
use super::SkillToolAuthority;
|
||||
use super::SkillToolAuthoritySelector;
|
||||
use super::SkillToolContext;
|
||||
use super::external_json_output;
|
||||
use super::is_bounded_handle;
|
||||
use super::pagination_cursor;
|
||||
use super::parse_args;
|
||||
use super::parse_pagination_cursor;
|
||||
use super::serialized_len;
|
||||
use super::skill_function_tool;
|
||||
use super::skill_json_output;
|
||||
use super::skill_tool_name;
|
||||
|
||||
const TOOL_NAME: &str = "list";
|
||||
const MAX_SKILLS_PER_PAGE: usize = 20;
|
||||
const MAX_LIST_RESPONSE_BYTES: usize = 512 * 1024;
|
||||
const OVERSIZED_ENTRY_WARNING: &str =
|
||||
"Some skills were omitted because their metadata is too large.";
|
||||
|
||||
#[derive(Deserialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
struct ListArgs {
|
||||
authority: SkillToolAuthority,
|
||||
authority: SkillToolAuthoritySelector,
|
||||
cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Eq, JsonSchema, PartialEq, Serialize)]
|
||||
#[derive(Clone, Debug, Eq, Hash, JsonSchema, PartialEq, Serialize)]
|
||||
#[schemars(deny_unknown_fields)]
|
||||
struct ListedSkill {
|
||||
authority: SkillToolAuthority,
|
||||
@@ -43,6 +55,7 @@ struct ListedSkill {
|
||||
struct ListResponse {
|
||||
skills: Vec<ListedSkill>,
|
||||
warnings: Vec<String>,
|
||||
next_cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -58,33 +71,79 @@ impl ToolExecutor<ToolCall> for ListTool {
|
||||
fn spec(&self) -> ToolSpec {
|
||||
skill_function_tool::<ListArgs, ListResponse>(
|
||||
TOOL_NAME,
|
||||
"List enabled skills owned by the requested authority. Only orchestrator-owned skills are currently supported. Returns the opaque package and main-resource handles required by skills.read.",
|
||||
"List skills owned by the requested authority. Returns the exact authority, package, and main_resource values required by skills.read. Pass next_cursor back as cursor to continue.",
|
||||
)
|
||||
}
|
||||
|
||||
fn handle(&self, call: ToolCall) -> ToolExecutorFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let args: ListArgs = parse_args(&call)?;
|
||||
let authority = args.authority.into_authority();
|
||||
let catalog = self.context.catalog(&call.turn_id, args.authority).await;
|
||||
let response = ListResponse {
|
||||
skills: catalog
|
||||
.entries
|
||||
.into_iter()
|
||||
.filter(|entry| entry.enabled && entry.authority == authority)
|
||||
.filter_map(listed_skill)
|
||||
.collect(),
|
||||
warnings: bounded_warnings(&catalog.warnings),
|
||||
let mut omitted_oversized_entry = false;
|
||||
let skills = catalog
|
||||
.entries
|
||||
.into_iter()
|
||||
.filter(|entry| {
|
||||
entry.is_model_visible() && args.authority.matches(&entry.authority)
|
||||
})
|
||||
.filter_map(|entry| {
|
||||
let listed = listed_skill(entry).filter(single_entry_response_is_bounded);
|
||||
omitted_oversized_entry |= listed.is_none();
|
||||
listed
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let start = parse_pagination_cursor(args.cursor.as_deref(), &skills, "skills.list")?;
|
||||
if start > skills.len() {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"skills.list cursor is invalid".to_string(),
|
||||
));
|
||||
}
|
||||
let mut warnings = if start == 0 {
|
||||
let mut warnings = catalog.warnings;
|
||||
if omitted_oversized_entry {
|
||||
warnings.push(OVERSIZED_ENTRY_WARNING.to_string());
|
||||
}
|
||||
bounded_warnings(&warnings)
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
external_json_output(&response)
|
||||
let mut end = (start + MAX_SKILLS_PER_PAGE).min(skills.len());
|
||||
loop {
|
||||
let response = ListResponse {
|
||||
skills: skills[start..end].to_vec(),
|
||||
warnings: warnings.clone(),
|
||||
next_cursor: (end < skills.len()).then(|| pagination_cursor(&skills, end)),
|
||||
};
|
||||
if serialized_len(&response)? <= MAX_LIST_RESPONSE_BYTES {
|
||||
return skill_json_output(&response, args.authority);
|
||||
}
|
||||
if end.saturating_sub(start) > 1 {
|
||||
end -= 1;
|
||||
} else if !warnings.is_empty() {
|
||||
warnings.clear();
|
||||
} else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"skill metadata is too large to list".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn single_entry_response_is_bounded(skill: &ListedSkill) -> bool {
|
||||
serialized_len(&ListResponse {
|
||||
skills: vec![skill.clone()],
|
||||
warnings: Vec::new(),
|
||||
next_cursor: Some(pagination_cursor(skill, usize::MAX)),
|
||||
})
|
||||
.is_ok_and(|size| size <= MAX_LIST_RESPONSE_BYTES)
|
||||
}
|
||||
|
||||
fn listed_skill(entry: SkillCatalogEntry) -> Option<ListedSkill> {
|
||||
let authority = SkillToolAuthority::from_authority(&entry.authority)?;
|
||||
if !is_bounded_handle(&entry.id.0, MAX_HANDLE_BYTES)
|
||||
if !is_bounded_handle(&entry.authority.id, MAX_HANDLE_BYTES)
|
||||
|| !is_bounded_handle(&entry.id.0, MAX_HANDLE_BYTES)
|
||||
|| !is_bounded_handle(entry.main_prompt.as_str(), MAX_HANDLE_BYTES)
|
||||
{
|
||||
return None;
|
||||
@@ -93,7 +152,7 @@ fn listed_skill(entry: SkillCatalogEntry) -> Option<ListedSkill> {
|
||||
Some(ListedSkill {
|
||||
authority,
|
||||
package: entry.id.0,
|
||||
name: entry.name,
|
||||
name: truncate_utf8_to_bytes(&entry.name, MAX_SKILL_NAME_BYTES).0,
|
||||
description: truncate_catalog_skill_description(&entry.description).into_owned(),
|
||||
main_resource: entry.main_prompt.as_str().to_string(),
|
||||
})
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
use std::collections::hash_map::DefaultHasher;
|
||||
use std::hash::Hash;
|
||||
use std::hash::Hasher;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::FunctionCallError;
|
||||
@@ -18,6 +21,7 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
use tokio::sync::OnceCell;
|
||||
|
||||
use crate::catalog::SkillAuthority;
|
||||
use crate::catalog::SkillCatalog;
|
||||
@@ -38,12 +42,17 @@ pub(crate) fn skill_tools(
|
||||
providers: SkillProviders,
|
||||
mcp_resources: Option<Arc<McpResourceClient>>,
|
||||
thread_state: Arc<SkillsThreadState>,
|
||||
orchestrator_available: bool,
|
||||
executor_query: Option<SkillListQuery>,
|
||||
shadow_selection: Arc<ShadowSelectionExperiment>,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
let context = SkillToolContext {
|
||||
providers,
|
||||
mcp_resources,
|
||||
thread_state,
|
||||
orchestrator_available,
|
||||
executor_query,
|
||||
executor_catalog: Arc::new(OnceCell::new()),
|
||||
shadow_selection,
|
||||
};
|
||||
vec![
|
||||
@@ -59,19 +68,26 @@ struct SkillToolContext {
|
||||
providers: SkillProviders,
|
||||
mcp_resources: Option<Arc<McpResourceClient>>,
|
||||
thread_state: Arc<SkillsThreadState>,
|
||||
orchestrator_available: bool,
|
||||
executor_query: Option<SkillListQuery>,
|
||||
executor_catalog: Arc<OnceCell<SkillCatalog>>,
|
||||
shadow_selection: Arc<ShadowSelectionExperiment>,
|
||||
}
|
||||
|
||||
impl SkillToolContext {
|
||||
async fn catalog(&self, turn_id: &str, authority: SkillToolAuthority) -> SkillCatalog {
|
||||
async fn catalog(&self, turn_id: &str, authority: SkillToolAuthoritySelector) -> SkillCatalog {
|
||||
match authority {
|
||||
SkillToolAuthority::Orchestrator => {
|
||||
SkillToolAuthoritySelector::Orchestrator => {
|
||||
if !self.orchestrator_available {
|
||||
return SkillCatalog::default();
|
||||
}
|
||||
self.thread_state
|
||||
.orchestrator_catalog_snapshot(
|
||||
self.mcp_resources.as_deref(),
|
||||
self.providers.list_orchestrator_for_turn(SkillListQuery {
|
||||
turn_id: turn_id.to_string(),
|
||||
executor_roots: Vec::new(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
host_snapshot: None,
|
||||
include_host_skills: false,
|
||||
include_bundled_skills: false,
|
||||
@@ -82,30 +98,73 @@ impl SkillToolContext {
|
||||
)
|
||||
.await
|
||||
}
|
||||
SkillToolAuthoritySelector::Executor => {
|
||||
let Some(mut query) = self.executor_query.clone() else {
|
||||
return SkillCatalog::default();
|
||||
};
|
||||
query.turn_id = turn_id.to_string();
|
||||
self.executor_catalog
|
||||
.get_or_init(|| self.providers.list_executor_for_turn(query))
|
||||
.await
|
||||
.clone()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
|
||||
#[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)]
|
||||
enum SkillToolAuthoritySelector {
|
||||
Orchestrator,
|
||||
Executor,
|
||||
}
|
||||
|
||||
impl SkillToolAuthoritySelector {
|
||||
fn matches(self, authority: &SkillAuthority) -> bool {
|
||||
match self {
|
||||
Self::Orchestrator => authority.kind == SkillSourceKind::Orchestrator,
|
||||
Self::Executor => authority.kind == SkillSourceKind::Executor,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize, Eq, Hash, JsonSchema, PartialEq, Serialize)]
|
||||
#[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)]
|
||||
enum SkillToolAuthority {
|
||||
Orchestrator,
|
||||
Executor { id: String },
|
||||
}
|
||||
|
||||
impl SkillToolAuthority {
|
||||
fn from_authority(authority: &SkillAuthority) -> Option<Self> {
|
||||
if authority
|
||||
!= &SkillAuthority::new(SkillSourceKind::Orchestrator, CODEX_APPS_MCP_SERVER_NAME)
|
||||
{
|
||||
return None;
|
||||
fn selector(&self) -> SkillToolAuthoritySelector {
|
||||
match self {
|
||||
Self::Orchestrator => SkillToolAuthoritySelector::Orchestrator,
|
||||
Self::Executor { .. } => SkillToolAuthoritySelector::Executor,
|
||||
}
|
||||
Some(Self::Orchestrator)
|
||||
}
|
||||
|
||||
fn into_authority(self) -> SkillAuthority {
|
||||
fn from_authority(authority: &SkillAuthority) -> Option<Self> {
|
||||
match &authority.kind {
|
||||
SkillSourceKind::Orchestrator if authority.id == CODEX_APPS_MCP_SERVER_NAME => {
|
||||
Some(Self::Orchestrator)
|
||||
}
|
||||
SkillSourceKind::Executor => Some(Self::Executor {
|
||||
id: authority.id.clone(),
|
||||
}),
|
||||
SkillSourceKind::Host | SkillSourceKind::Orchestrator | SkillSourceKind::Custom(_) => {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn matches(&self, authority: &SkillAuthority) -> bool {
|
||||
match self {
|
||||
Self::Orchestrator => {
|
||||
SkillAuthority::new(SkillSourceKind::Orchestrator, CODEX_APPS_MCP_SERVER_NAME)
|
||||
authority.kind == SkillSourceKind::Orchestrator
|
||||
&& authority.id == CODEX_APPS_MCP_SERVER_NAME
|
||||
}
|
||||
Self::Executor { id } => {
|
||||
authority.kind == SkillSourceKind::Executor && authority.id == *id
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -158,9 +217,50 @@ fn is_bounded_handle(value: &str, max_bytes: usize) -> bool {
|
||||
!value.is_empty() && value.len() <= max_bytes && !value.chars().any(char::is_control)
|
||||
}
|
||||
|
||||
fn external_json_output<T: Serialize>(value: &T) -> Result<Box<dyn ToolOutput>, FunctionCallError> {
|
||||
fn pagination_cursor(value: &(impl Hash + ?Sized), offset: usize) -> String {
|
||||
format!("{:016x}:{offset}", value_fingerprint(value))
|
||||
}
|
||||
|
||||
fn parse_pagination_cursor(
|
||||
cursor: Option<&str>,
|
||||
value: &(impl Hash + ?Sized),
|
||||
tool: &str,
|
||||
) -> Result<usize, FunctionCallError> {
|
||||
let Some(cursor) = cursor else {
|
||||
return Ok(0);
|
||||
};
|
||||
let invalid = || FunctionCallError::RespondToModel(format!("{tool} cursor is invalid"));
|
||||
let (fingerprint, offset) = cursor.split_once(':').ok_or_else(invalid)?;
|
||||
if u64::from_str_radix(fingerprint, 16).ok() != Some(value_fingerprint(value)) {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"{tool} cursor is stale; restart from the first page"
|
||||
)));
|
||||
}
|
||||
offset.parse::<usize>().map_err(|_| invalid())
|
||||
}
|
||||
|
||||
fn value_fingerprint(value: &(impl Hash + ?Sized)) -> u64 {
|
||||
let mut hasher = DefaultHasher::new();
|
||||
value.hash(&mut hasher);
|
||||
hasher.finish()
|
||||
}
|
||||
|
||||
fn serialized_len(value: &impl Serialize) -> Result<usize, FunctionCallError> {
|
||||
serde_json::to_vec(value)
|
||||
.map(|value| value.len())
|
||||
.map_err(|err| FunctionCallError::Fatal(err.to_string()))
|
||||
}
|
||||
|
||||
fn skill_json_output<T: Serialize>(
|
||||
value: &T,
|
||||
authority: SkillToolAuthoritySelector,
|
||||
) -> Result<Box<dyn ToolOutput>, FunctionCallError> {
|
||||
let value = serde_json::to_value(value).map_err(|err| {
|
||||
FunctionCallError::Fatal(format!("failed to serialize tool output: {err}"))
|
||||
})?;
|
||||
Ok(Box::new(JsonToolOutput::new(value).with_external_context()))
|
||||
let output = JsonToolOutput::new(value);
|
||||
Ok(match authority {
|
||||
SkillToolAuthoritySelector::Orchestrator => Box::new(output.with_external_context()),
|
||||
SkillToolAuthoritySelector::Executor => Box::new(output),
|
||||
})
|
||||
}
|
||||
@@ -8,20 +8,23 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::catalog::SkillPackageId;
|
||||
use crate::catalog::SkillResourceId;
|
||||
use crate::provider::SkillReadRequest;
|
||||
|
||||
use super::MAX_HANDLE_BYTES;
|
||||
use super::SkillToolAuthority;
|
||||
use super::SkillToolContext;
|
||||
use super::external_json_output;
|
||||
use super::pagination_cursor;
|
||||
use super::parse_args;
|
||||
use super::parse_pagination_cursor;
|
||||
use super::serialized_len;
|
||||
use super::skill_function_tool;
|
||||
use super::skill_json_output;
|
||||
use super::skill_tool_name;
|
||||
use super::validate_handle;
|
||||
|
||||
const TOOL_NAME: &str = "read";
|
||||
const MAX_READ_RESPONSE_BYTES: usize = 512 * 1024;
|
||||
|
||||
#[derive(Deserialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
@@ -29,6 +32,7 @@ struct ReadArgs {
|
||||
authority: SkillToolAuthority,
|
||||
package: String,
|
||||
resource: String,
|
||||
cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Eq, JsonSchema, PartialEq, Serialize)]
|
||||
@@ -36,6 +40,7 @@ struct ReadArgs {
|
||||
struct ReadResponse {
|
||||
resource: String,
|
||||
contents: String,
|
||||
next_cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -51,28 +56,46 @@ impl ToolExecutor<ToolCall> for ReadTool {
|
||||
fn spec(&self) -> ToolSpec {
|
||||
skill_function_tool::<ReadArgs, ReadResponse>(
|
||||
TOOL_NAME,
|
||||
"Read one complete resource from an enabled skill. Pass the exact authority and package returned by skills.list; resource identifiers remain opaque and are routed to that authority.",
|
||||
"Read one page from a skill resource. Pass the exact authority and package from skills.list, plus its main_resource or a referenced resource beneath that package. Pass next_cursor back as cursor to continue.",
|
||||
)
|
||||
}
|
||||
|
||||
fn handle(&self, call: ToolCall) -> ToolExecutorFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let args: ReadArgs = parse_args(&call)?;
|
||||
let authority = args.authority.into_authority();
|
||||
if let SkillToolAuthority::Executor { id } = &args.authority {
|
||||
validate_handle("authority.id", id, MAX_HANDLE_BYTES)?;
|
||||
}
|
||||
validate_handle("package", &args.package, MAX_HANDLE_BYTES)?;
|
||||
validate_handle("resource", &args.resource, MAX_HANDLE_BYTES)?;
|
||||
|
||||
let catalog = self.context.catalog(&call.turn_id, args.authority).await;
|
||||
let output_authority = args.authority.selector();
|
||||
let catalog = self.context.catalog(&call.turn_id, output_authority).await;
|
||||
let Some(skill_entry) = catalog.entries.iter().find(|entry| {
|
||||
entry.enabled && entry.authority == authority && entry.id.0 == args.package
|
||||
entry.enabled
|
||||
&& args.authority.matches(&entry.authority)
|
||||
&& entry.id.0 == args.package
|
||||
}) else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"skill package is not available from the requested authority".to_string(),
|
||||
));
|
||||
};
|
||||
let authority = skill_entry.authority.clone();
|
||||
let package = skill_entry.id.clone();
|
||||
let main_prompt = skill_entry.main_prompt.clone();
|
||||
|
||||
let requested_resource = SkillResourceId::new(args.resource);
|
||||
let requested_resource = if args.resource == main_prompt.as_str() {
|
||||
main_prompt.clone()
|
||||
} else {
|
||||
main_prompt
|
||||
.bind_environment_package_resource(&package, args.resource.clone())
|
||||
.unwrap_or_else(|| SkillResourceId::new(args.resource))
|
||||
};
|
||||
let resolved_executor_roots = self
|
||||
.context
|
||||
.executor_query
|
||||
.as_ref()
|
||||
.map(|query| query.resolved_executor_roots.clone())
|
||||
.unwrap_or_default();
|
||||
let result = self
|
||||
.context
|
||||
.thread_state
|
||||
@@ -80,8 +103,9 @@ impl ToolExecutor<ToolCall> for ReadTool {
|
||||
&self.context.providers,
|
||||
SkillReadRequest {
|
||||
authority,
|
||||
package: SkillPackageId(args.package),
|
||||
package,
|
||||
resource: requested_resource.clone(),
|
||||
resolved_executor_roots,
|
||||
host_snapshot: None,
|
||||
mcp_resources: self.context.mcp_resources.clone(),
|
||||
},
|
||||
@@ -102,21 +126,60 @@ impl ToolExecutor<ToolCall> for ReadTool {
|
||||
"skill provider returned a different resource".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
if let Some(state) = self
|
||||
.context
|
||||
.thread_state
|
||||
.shadow_selection_turn(&call.turn_id)
|
||||
if output_authority == super::SkillToolAuthoritySelector::Orchestrator
|
||||
&& let Some(state) = self
|
||||
.context
|
||||
.thread_state
|
||||
.shadow_selection_turn(&call.turn_id)
|
||||
{
|
||||
self.context
|
||||
.shadow_selection
|
||||
.record_invocation(&state, main_prompt.as_str());
|
||||
}
|
||||
|
||||
external_json_output(&ReadResponse {
|
||||
resource: result.resource.as_str().to_string(),
|
||||
contents: result.contents,
|
||||
})
|
||||
let start = parse_pagination_cursor(
|
||||
args.cursor.as_deref(),
|
||||
result.contents.as_str(),
|
||||
"skills.read",
|
||||
)?;
|
||||
if start > result.contents.len() || !result.contents.is_char_boundary(start) {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"skills.read cursor is invalid".to_string(),
|
||||
));
|
||||
}
|
||||
let response = page_response(result.resource.as_str(), &result.contents, start)?;
|
||||
skill_json_output(&response, output_authority)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn page_response(
|
||||
resource: &str,
|
||||
contents: &str,
|
||||
start: usize,
|
||||
) -> Result<ReadResponse, FunctionCallError> {
|
||||
let response = |end, next_cursor| ReadResponse {
|
||||
resource: resource.to_string(),
|
||||
contents: contents[start..end].to_string(),
|
||||
next_cursor,
|
||||
};
|
||||
let complete = response(contents.len(), None);
|
||||
if serialized_len(&complete)? <= MAX_READ_RESPONSE_BYTES {
|
||||
return Ok(complete);
|
||||
}
|
||||
|
||||
let mut end = contents.len();
|
||||
while end > start {
|
||||
end = start + (end - start) / 2;
|
||||
while !contents.is_char_boundary(end) {
|
||||
end -= 1;
|
||||
}
|
||||
let candidate = response(end, Some(pagination_cursor(contents, end)));
|
||||
if serialized_len(&candidate)? <= MAX_READ_RESPONSE_BYTES {
|
||||
return Ok(candidate);
|
||||
}
|
||||
}
|
||||
Err(FunctionCallError::Fatal(
|
||||
"skill resource handle leaves no room for contents".to_string(),
|
||||
))
|
||||
}
|
||||
Reference in new issue
Block a user