mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Add batched executor capability discovery (#33852)
## Why Selected capability roots can contribute plugins, MCP servers, connectors, and skills. Discovering each contribution separately requires repeated access to the executor filesystem. ## What changed - Add the `capabilityRoots/discoverV1` exec-server RPC to scan selected roots and materialize recognized plugin manifests, configuration files, skill instructions, and skill metadata in one bounded request. - Add the opt-in `executor_capability_discovery` feature, with a thread-scoped cache and per-step snapshot shared by MCP and skill discovery. - Parse MCP, connector, and skill contributions from the materialized snapshot, including serving cached skill instructions without another filesystem read. ## Testing - Cover discovery limits, manifest precedence, root-local failures, cache reuse, plugin contributions, and parity with the existing environment skill loader. GitOrigin-RevId: f98fd2321cafb58c596db02da1f83c09d8eb375d
This commit is contained in:
1 parent
caa6e554d6
commit
08e30a2e4e
47 files changed
+1957
-47
No files matched your search
@@ -1,5 +1,6 @@
|
||||
use codex_core_skills::model::SkillDependencies;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// Source authority that owns a skill package and must be used to read it.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
|
||||
@@ -83,6 +84,23 @@ impl SkillResourceId {
|
||||
environment_path: Some(EnvironmentSkillResource {
|
||||
environment_id: environment_id.into(),
|
||||
path,
|
||||
contents: None,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn environment_with_contents(
|
||||
id: impl Into<String>,
|
||||
environment_id: impl Into<String>,
|
||||
path: PathUri,
|
||||
contents: String,
|
||||
) -> Self {
|
||||
Self {
|
||||
id: id.into(),
|
||||
environment_path: Some(EnvironmentSkillResource {
|
||||
environment_id: environment_id.into(),
|
||||
path,
|
||||
contents: Some(contents.into()),
|
||||
}),
|
||||
}
|
||||
}
|
||||
@@ -96,12 +114,19 @@ impl SkillResourceId {
|
||||
.as_ref()
|
||||
.map(|resource| (resource.environment_id.as_str(), &resource.path))
|
||||
}
|
||||
|
||||
pub(crate) fn environment_contents(&self) -> Option<&str> {
|
||||
self.environment_path
|
||||
.as_ref()
|
||||
.and_then(|resource| resource.contents.as_deref())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
|
||||
struct EnvironmentSkillResource {
|
||||
environment_id: String,
|
||||
path: PathUri,
|
||||
contents: Option<Arc<str>>,
|
||||
}
|
||||
|
||||
/// Metadata shown in the always-visible skills catalog.
|
||||
|
||||
@@ -132,6 +132,7 @@ where
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
|
||||
mcp_resources: session_store.get::<McpResourceClient>(),
|
||||
executor_capability_discovery: None,
|
||||
},
|
||||
&thread_state,
|
||||
)
|
||||
@@ -169,6 +170,7 @@ where
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
include_orchestrator_skills: false,
|
||||
mcp_resources: input.session_store.get::<McpResourceClient>(),
|
||||
executor_capability_discovery: input.executor_capability_discovery.cloned(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -285,6 +287,7 @@ where
|
||||
include_bundled_skills: config.bundled_skills_enabled,
|
||||
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
|
||||
mcp_resources: session_store.get::<McpResourceClient>(),
|
||||
executor_capability_discovery: None,
|
||||
};
|
||||
let host_query = query.clone();
|
||||
let mut catalog = turn_store
|
||||
|
||||
@@ -7,6 +7,7 @@ mod host;
|
||||
mod orchestrator;
|
||||
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_exec_server::ExecutorCapabilityDiscoverySnapshot;
|
||||
use codex_mcp::McpResourceClient;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
|
||||
@@ -31,6 +32,8 @@ pub struct SkillListQuery {
|
||||
pub include_bundled_skills: bool,
|
||||
pub include_orchestrator_skills: bool,
|
||||
pub mcp_resources: Option<Arc<McpResourceClient>>,
|
||||
/// Present only when the opt-in high-level executor discovery path is selected.
|
||||
pub executor_capability_discovery: Option<ExecutorCapabilityDiscoverySnapshot>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core_skills::loader::EnvironmentSkillMetadata;
|
||||
use codex_core_skills::loader::load_environment_skills_from_discovery;
|
||||
use codex_core_skills::loader::load_environment_skills_from_root;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
@@ -44,6 +45,9 @@ impl ExecutorSkillProvider {
|
||||
impl SkillProvider for ExecutorSkillProvider {
|
||||
fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> {
|
||||
Box::pin(async move {
|
||||
if let Some(discovery) = query.executor_capability_discovery {
|
||||
return Ok(self.list_from_discovery(&discovery));
|
||||
}
|
||||
let mut catalog = SkillCatalog::default();
|
||||
for selected_root in query.executor_roots {
|
||||
let selected_root_id = selected_root.id;
|
||||
@@ -74,6 +78,7 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
authority.clone(),
|
||||
&selected_root_id,
|
||||
&environment_id,
|
||||
/*instructions*/ None,
|
||||
));
|
||||
}
|
||||
}
|
||||
@@ -95,6 +100,12 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
"executor skill resource does not match its package",
|
||||
));
|
||||
}
|
||||
if let Some(contents) = request.resource.environment_contents() {
|
||||
return Ok(SkillReadResult {
|
||||
resource: request.resource.clone(),
|
||||
contents: contents.to_string(),
|
||||
});
|
||||
}
|
||||
let Some((environment_id, resource_path)) = request.resource.environment_path() else {
|
||||
return Err(SkillProviderError::new(
|
||||
"executor skill resource is not bound to an environment",
|
||||
@@ -128,11 +139,50 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecutorSkillProvider {
|
||||
fn list_from_discovery(
|
||||
&self,
|
||||
snapshot: &codex_exec_server::ExecutorCapabilityDiscoverySnapshot,
|
||||
) -> SkillCatalog {
|
||||
let mut catalog = SkillCatalog::default();
|
||||
for root in snapshot.roots() {
|
||||
let selected_root_id = &root.selected_root.id;
|
||||
let CapabilityRootLocation::Environment { environment_id, .. } =
|
||||
&root.selected_root.location;
|
||||
let discovery = match &root.result {
|
||||
Ok(discovery) => discovery.as_ref(),
|
||||
Err(error) => {
|
||||
catalog.warnings.push(format!(
|
||||
"Selected capability root `{selected_root_id}` discovery failed: {error}"
|
||||
));
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let outcome =
|
||||
load_environment_skills_from_discovery(discovery, self.restriction_product);
|
||||
catalog.warnings.extend(outcome.warnings);
|
||||
let authority =
|
||||
SkillAuthority::new(SkillSourceKind::Executor, selected_root_id.clone());
|
||||
for skill in outcome.skills {
|
||||
catalog.push_entry(catalog_entry_from_skill(
|
||||
&skill.metadata,
|
||||
authority.clone(),
|
||||
selected_root_id,
|
||||
environment_id,
|
||||
Some(skill.instructions),
|
||||
));
|
||||
}
|
||||
}
|
||||
catalog
|
||||
}
|
||||
}
|
||||
|
||||
fn catalog_entry_from_skill(
|
||||
skill: &EnvironmentSkillMetadata,
|
||||
authority: SkillAuthority,
|
||||
selected_root_id: &str,
|
||||
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() {
|
||||
@@ -143,16 +193,25 @@ fn catalog_entry_from_skill(
|
||||
"skill://{selected_root_id}/{}",
|
||||
normalized_path.trim_start_matches('/')
|
||||
);
|
||||
let main_prompt = match instructions {
|
||||
Some(contents) => SkillResourceId::environment_with_contents(
|
||||
display_path.clone(),
|
||||
environment_id,
|
||||
skill.path_to_skills_md.clone(),
|
||||
contents,
|
||||
),
|
||||
None => SkillResourceId::environment(
|
||||
display_path.clone(),
|
||||
environment_id,
|
||||
skill.path_to_skills_md.clone(),
|
||||
),
|
||||
};
|
||||
let entry = SkillCatalogEntry::new(
|
||||
SkillPackageId(display_path.clone()),
|
||||
authority,
|
||||
skill.name.clone(),
|
||||
skill.description.clone(),
|
||||
SkillResourceId::environment(
|
||||
display_path.clone(),
|
||||
environment_id,
|
||||
skill.path_to_skills_md.clone(),
|
||||
),
|
||||
main_prompt,
|
||||
)
|
||||
.with_short_description(skill.short_description.clone())
|
||||
.with_display_path(display_path)
|
||||
|
||||
@@ -30,6 +30,7 @@ pub(crate) struct SkillsThreadState {
|
||||
config: Mutex<SkillsExtensionConfig>,
|
||||
orchestrator_skills_available: bool,
|
||||
executor_cache: Mutex<Vec<CachedExecutorCatalog>>,
|
||||
executor_discovery_cache: Mutex<Option<CachedExecutorDiscoveryCatalog>>,
|
||||
orchestrator_cache: Mutex<Option<Arc<OrchestratorGenerationCache>>>,
|
||||
shadow_selection_turn: Mutex<Option<ShadowSelectionTurn>>,
|
||||
}
|
||||
@@ -40,6 +41,7 @@ impl SkillsThreadState {
|
||||
config: Mutex::new(config),
|
||||
orchestrator_skills_available,
|
||||
executor_cache: Mutex::new(Vec::new()),
|
||||
executor_discovery_cache: Mutex::new(None),
|
||||
orchestrator_cache: Mutex::new(None),
|
||||
shadow_selection_turn: Mutex::new(None),
|
||||
}
|
||||
@@ -107,6 +109,11 @@ impl SkillsThreadState {
|
||||
providers: &SkillProviders,
|
||||
mut query: SkillListQuery,
|
||||
) -> SkillCatalog {
|
||||
if query.executor_capability_discovery.is_some() {
|
||||
return self
|
||||
.executor_discovery_catalog_snapshot(providers, query)
|
||||
.await;
|
||||
}
|
||||
let roots = std::mem::take(&mut query.executor_roots);
|
||||
let mut catalog = SkillCatalog::default();
|
||||
for root in roots {
|
||||
@@ -119,6 +126,36 @@ impl SkillsThreadState {
|
||||
catalog
|
||||
}
|
||||
|
||||
async fn executor_discovery_catalog_snapshot(
|
||||
&self,
|
||||
providers: &SkillProviders,
|
||||
query: SkillListQuery,
|
||||
) -> SkillCatalog {
|
||||
if let Some(cached) = self
|
||||
.executor_discovery_cache
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.as_ref()
|
||||
.filter(|cached| cached.roots == query.executor_roots)
|
||||
{
|
||||
return cached.catalog.clone();
|
||||
}
|
||||
let roots = query.executor_roots.clone();
|
||||
let discovered = providers.list_executor_for_turn(query).await;
|
||||
let mut cache = self
|
||||
.executor_discovery_cache
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
if let Some(cached) = cache.as_ref().filter(|cached| cached.roots == roots) {
|
||||
return cached.catalog.clone();
|
||||
}
|
||||
*cache = Some(CachedExecutorDiscoveryCatalog {
|
||||
roots,
|
||||
catalog: discovered.clone(),
|
||||
});
|
||||
discovered
|
||||
}
|
||||
|
||||
pub(crate) async fn orchestrator_catalog_snapshot(
|
||||
&self,
|
||||
mcp_resources: Option<&McpResourceClient>,
|
||||
@@ -236,6 +273,11 @@ struct CachedExecutorCatalog {
|
||||
catalog: SkillCatalog,
|
||||
}
|
||||
|
||||
struct CachedExecutorDiscoveryCatalog {
|
||||
roots: Vec<SelectedCapabilityRoot>,
|
||||
catalog: SkillCatalog,
|
||||
}
|
||||
|
||||
struct OrchestratorGenerationCache {
|
||||
mcp_cache_key: Option<McpResourceClientCacheKey>,
|
||||
catalog: OnceCell<SkillCatalog>,
|
||||
|
||||
@@ -77,6 +77,7 @@ impl SkillToolContext {
|
||||
include_bundled_skills: false,
|
||||
include_orchestrator_skills: true,
|
||||
mcp_resources: self.mcp_resources.clone(),
|
||||
executor_capability_discovery: None,
|
||||
}),
|
||||
)
|
||||
.await
|
||||
|
||||
Reference in new issue
Block a user