mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Add shared skill root loading interfaces (#37439)
## What changed - Add `SkillRootLoader` and its request/result types to `codex-skills` so skill roots can be loaded through a shared, object-safe interface. - Add owner-managed snapshot cache handles that support reuse and use cache allocation identity for equality and hashing. - Move `SkillError` into `codex-skills` and re-export the new loading API. ## Testing - Verify snapshot sharing and identity semantics. - Verify loaders can be used as shared trait objects. GitOrigin-RevId: a26a8101008a6c862824a7346cdbe6d05f4bcc01
This commit is contained in:
1 parent
a7dcd20d38
commit
a4b129eb3e
4 files changed
+208
-6
No files matched your search
@@ -8,6 +8,7 @@ use codex_exec_server::ExecutorFileSystem;
|
|||||||
use codex_exec_server::LOCAL_FS;
|
use codex_exec_server::LOCAL_FS;
|
||||||
use codex_protocol::protocol::Product;
|
use codex_protocol::protocol::Product;
|
||||||
pub use codex_skills::SkillDependencies;
|
pub use codex_skills::SkillDependencies;
|
||||||
|
pub use codex_skills::SkillError;
|
||||||
pub use codex_skills::SkillInterface;
|
pub use codex_skills::SkillInterface;
|
||||||
pub use codex_skills::SkillMetadata;
|
pub use codex_skills::SkillMetadata;
|
||||||
pub use codex_skills::SkillPolicy;
|
pub use codex_skills::SkillPolicy;
|
||||||
@@ -15,12 +16,6 @@ pub use codex_skills::SkillToolDependency;
|
|||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use codex_utils_path_uri::PathUri;
|
use codex_utils_path_uri::PathUri;
|
||||||
|
|
||||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
|
||||||
pub struct SkillError {
|
|
||||||
pub path: AbsolutePathBuf,
|
|
||||||
pub message: String,
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Debug, Clone, Default)]
|
#[derive(Debug, Clone, Default)]
|
||||||
pub struct SkillLoadOutcome {
|
pub struct SkillLoadOutcome {
|
||||||
pub skills: Vec<SkillMetadata>,
|
pub skills: Vec<SkillMetadata>,
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
mod interface;
|
mod interface;
|
||||||
mod invocation;
|
mod invocation;
|
||||||
|
mod loading;
|
||||||
mod mentions;
|
mod mentions;
|
||||||
mod model;
|
mod model;
|
||||||
mod name_counts;
|
mod name_counts;
|
||||||
@@ -12,6 +13,14 @@ pub use interface::SkillInterfaceFile;
|
|||||||
pub use interface::resolve_skill_interface;
|
pub use interface::resolve_skill_interface;
|
||||||
pub use invocation::ImplicitSkillLookup;
|
pub use invocation::ImplicitSkillLookup;
|
||||||
pub use invocation::detect_implicit_skill_invocation_for_command;
|
pub use invocation::detect_implicit_skill_invocation_for_command;
|
||||||
|
pub use loading::LoadedSkillRoot;
|
||||||
|
pub use loading::LoadedSkills;
|
||||||
|
pub use loading::SkillError;
|
||||||
|
pub use loading::SkillLoadFuture;
|
||||||
|
pub use loading::SkillRootLoadRequest;
|
||||||
|
pub use loading::SkillRootLoader;
|
||||||
|
pub use loading::SkillRootSnapshotCache;
|
||||||
|
pub use loading::SkillRootSnapshots;
|
||||||
pub use mentions::ToolMentionKind;
|
pub use mentions::ToolMentionKind;
|
||||||
pub use mentions::ToolMentions;
|
pub use mentions::ToolMentions;
|
||||||
pub use mentions::app_id_from_path;
|
pub use mentions::app_id_from_path;
|
||||||
|
|||||||
@@ -0,0 +1,119 @@
|
|||||||
|
use std::collections::HashMap;
|
||||||
|
use std::fmt;
|
||||||
|
use std::future::Future;
|
||||||
|
use std::hash::Hash;
|
||||||
|
use std::hash::Hasher;
|
||||||
|
use std::pin::Pin;
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
use codex_protocol::protocol::Product;
|
||||||
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
|
|
||||||
|
use crate::SkillMetadata;
|
||||||
|
|
||||||
|
/// A skill document that could not be read, parsed, or validated.
|
||||||
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
|
pub struct SkillError {
|
||||||
|
pub path: AbsolutePathBuf,
|
||||||
|
pub message: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Filesystem-independent metadata discovered beneath one canonical skill root.
|
||||||
|
#[derive(Debug, Clone, PartialEq)]
|
||||||
|
pub struct LoadedSkillRoot {
|
||||||
|
pub root: AbsolutePathBuf,
|
||||||
|
pub skills: Vec<SkillMetadata>,
|
||||||
|
pub skill_discovery_path_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||||
|
pub errors: Vec<SkillError>,
|
||||||
|
pub is_agent_plugin: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Skills and errors produced by loading an ordered set of roots.
|
||||||
|
#[derive(Debug, Clone, Default, PartialEq)]
|
||||||
|
pub struct LoadedSkills {
|
||||||
|
pub skills: Vec<SkillMetadata>,
|
||||||
|
pub errors: Vec<SkillError>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Caches parsed roots without assigning ownership of the cache to a loader.
|
||||||
|
///
|
||||||
|
/// Implementations own storage and determine its lifetime; loaders may reuse an existing snapshot
|
||||||
|
/// or publish a newly loaded one without learning about the caller's broader state.
|
||||||
|
pub trait SkillRootSnapshotCache<Root>: Send + Sync {
|
||||||
|
fn get(&self, root: &Root) -> Option<LoadedSkillRoot>;
|
||||||
|
|
||||||
|
fn insert(&self, root: Root, snapshot: LoadedSkillRoot);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Shared access to one owner-managed collection of parsed skill roots.
|
||||||
|
///
|
||||||
|
/// Equality and hashing use the cache allocation's identity so callers can include the handle in
|
||||||
|
/// their own cache keys without inspecting or taking ownership of its contents.
|
||||||
|
pub struct SkillRootSnapshots<Root> {
|
||||||
|
cache: Arc<dyn SkillRootSnapshotCache<Root>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<Root> SkillRootSnapshots<Root> {
|
||||||
|
pub fn new(cache: Arc<dyn SkillRootSnapshotCache<Root>>) -> Self {
|
||||||
|
Self { cache }
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn get(&self, root: &Root) -> Option<LoadedSkillRoot> {
|
||||||
|
self.cache.get(root)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn insert(&self, root: Root, snapshot: LoadedSkillRoot) {
|
||||||
|
self.cache.insert(root, snapshot);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<Root> Clone for SkillRootSnapshots<Root> {
|
||||||
|
fn clone(&self) -> Self {
|
||||||
|
Self {
|
||||||
|
cache: Arc::clone(&self.cache),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<Root> PartialEq for SkillRootSnapshots<Root> {
|
||||||
|
fn eq(&self, other: &Self) -> bool {
|
||||||
|
Arc::ptr_eq(&self.cache, &other.cache)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<Root> Eq for SkillRootSnapshots<Root> {}
|
||||||
|
|
||||||
|
impl<Root> Hash for SkillRootSnapshots<Root> {
|
||||||
|
fn hash<H: Hasher>(&self, state: &mut H) {
|
||||||
|
(Arc::as_ptr(&self.cache) as *const ()).hash(state);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<Root> fmt::Debug for SkillRootSnapshots<Root> {
|
||||||
|
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||||
|
f.debug_struct("SkillRootSnapshots").finish_non_exhaustive()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Inputs for loading roots while optionally reusing snapshots owned by the caller.
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub struct SkillRootLoadRequest<Root> {
|
||||||
|
pub roots: Vec<Root>,
|
||||||
|
pub restriction_product: Option<Product>,
|
||||||
|
pub snapshots: Option<SkillRootSnapshots<Root>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Object-safe future returned by a shared skill-root loader.
|
||||||
|
pub type SkillLoadFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;
|
||||||
|
|
||||||
|
/// Loads ordered skill roots without owning their source inventory or capability state.
|
||||||
|
///
|
||||||
|
/// Implementations are responsible for filesystem access, metadata parsing, root precedence,
|
||||||
|
/// product filtering, and reuse of any snapshots supplied in the request.
|
||||||
|
pub trait SkillRootLoader<Root>: Send + Sync {
|
||||||
|
fn load_roots(&self, request: SkillRootLoadRequest<Root>) -> SkillLoadFuture<'_, LoadedSkills>;
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
#[path = "loading_tests.rs"]
|
||||||
|
mod tests;
|
||||||
@@ -0,0 +1,79 @@
|
|||||||
|
use std::collections::HashMap;
|
||||||
|
use std::collections::HashSet;
|
||||||
|
use std::sync::Arc;
|
||||||
|
use std::sync::Mutex;
|
||||||
|
|
||||||
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
|
use pretty_assertions::assert_eq;
|
||||||
|
|
||||||
|
use super::LoadedSkillRoot;
|
||||||
|
use super::LoadedSkills;
|
||||||
|
use super::SkillLoadFuture;
|
||||||
|
use super::SkillRootLoadRequest;
|
||||||
|
use super::SkillRootLoader;
|
||||||
|
use super::SkillRootSnapshotCache;
|
||||||
|
use super::SkillRootSnapshots;
|
||||||
|
|
||||||
|
#[derive(Default)]
|
||||||
|
struct TestSnapshotCache {
|
||||||
|
snapshots: Mutex<HashMap<String, LoadedSkillRoot>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl SkillRootSnapshotCache<String> for TestSnapshotCache {
|
||||||
|
fn get(&self, root: &String) -> Option<LoadedSkillRoot> {
|
||||||
|
self.snapshots.lock().unwrap().get(root).cloned()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert(&self, root: String, snapshot: LoadedSkillRoot) {
|
||||||
|
self.snapshots.lock().unwrap().insert(root, snapshot);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
struct TestSkillRootLoader;
|
||||||
|
|
||||||
|
impl SkillRootLoader<String> for TestSkillRootLoader {
|
||||||
|
fn load_roots(
|
||||||
|
&self,
|
||||||
|
_request: SkillRootLoadRequest<String>,
|
||||||
|
) -> SkillLoadFuture<'_, LoadedSkills> {
|
||||||
|
Box::pin(async { LoadedSkills::default() })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn snapshot_handles_share_owner_managed_values_and_identity() {
|
||||||
|
let snapshots = SkillRootSnapshots::new(Arc::new(TestSnapshotCache::default()));
|
||||||
|
let cloned_snapshots = snapshots.clone();
|
||||||
|
let separate_snapshots = SkillRootSnapshots::new(Arc::new(TestSnapshotCache::default()));
|
||||||
|
let root = "plugin-root".to_string();
|
||||||
|
let snapshot = LoadedSkillRoot {
|
||||||
|
root: AbsolutePathBuf::try_from(std::env::temp_dir()).unwrap(),
|
||||||
|
skills: Vec::new(),
|
||||||
|
skill_discovery_path_by_path: Arc::new(HashMap::new()),
|
||||||
|
errors: Vec::new(),
|
||||||
|
is_agent_plugin: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
snapshots.insert(root.clone(), snapshot.clone());
|
||||||
|
|
||||||
|
assert_eq!(cloned_snapshots.get(&root), Some(snapshot));
|
||||||
|
assert_eq!(separate_snapshots.get(&root), None);
|
||||||
|
assert_eq!(snapshots, cloned_snapshots);
|
||||||
|
assert_ne!(snapshots, separate_snapshots);
|
||||||
|
assert_eq!(
|
||||||
|
HashSet::from([snapshots, cloned_snapshots, separate_snapshots]).len(),
|
||||||
|
2
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn skill_root_loader_supports_shared_trait_objects() {
|
||||||
|
let loader: Arc<dyn SkillRootLoader<String>> = Arc::new(TestSkillRootLoader);
|
||||||
|
let future = loader.load_roots(SkillRootLoadRequest {
|
||||||
|
roots: vec!["plugin-root".to_string()],
|
||||||
|
restriction_product: None,
|
||||||
|
snapshots: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
drop(future);
|
||||||
|
}
|
||||||
Reference in new issue
Block a user