From 2d385e166c2dff131ca21294216ce61eb62fd3c2 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 3 Jun 2026 01:10:26 +0200 Subject: [PATCH] feat: add skills extension scaffold (#25953) ## Disclaimer This is only here for iteration purpose! Do not make any code rely on this ## Why Skills still live behind `codex-core` discovery and injection paths, but the extension system needs an authority-aware home before that logic can move. This adds that boundary without changing current skills behavior, and keeps host, executor, and remote skills distinct so future list/read/search flows do not collapse back to ambient local paths. ## What changed - Add the `codex-skills-extension` workspace/Bazel crate under `ext/skills`. - Define the initial catalog, authority, provider, and turn-state types for authority-bound skill packages and resources. - Register placeholder thread/config/prompt/turn lifecycle contributors plus host, executor, and remote provider aggregation points. - Capture the remaining extraction work as TODOs, including the missing extension API hooks needed for per-turn catalog construction and typed skill injection. - Keep plugins outside the runtime skills model: plugin-installed skills are treated as materialized host-owned skill sources once available. ## Verification - Not run locally. --- codex-rs/Cargo.lock | 9 ++ codex-rs/Cargo.toml | 1 + codex-rs/ext/skills/BUILD.bazel | 6 + codex-rs/ext/skills/Cargo.toml | 19 +++ codex-rs/ext/skills/src/catalog.rs | 86 +++++++++++ codex-rs/ext/skills/src/extension.rs | 140 ++++++++++++++++++ codex-rs/ext/skills/src/lib.rs | 7 + codex-rs/ext/skills/src/provider.rs | 63 ++++++++ codex-rs/ext/skills/src/providers/executor.rs | 56 +++++++ codex-rs/ext/skills/src/providers/host.rs | 65 ++++++++ codex-rs/ext/skills/src/providers/mod.rs | 50 +++++++ codex-rs/ext/skills/src/providers/remote.rs | 58 ++++++++ codex-rs/ext/skills/src/state.rs | 24 +++ 13 files changed, 584 insertions(+) create mode 100644 codex-rs/ext/skills/BUILD.bazel create mode 100644 codex-rs/ext/skills/Cargo.toml create mode 100644 codex-rs/ext/skills/src/catalog.rs create mode 100644 codex-rs/ext/skills/src/extension.rs create mode 100644 codex-rs/ext/skills/src/lib.rs create mode 100644 codex-rs/ext/skills/src/provider.rs create mode 100644 codex-rs/ext/skills/src/providers/executor.rs create mode 100644 codex-rs/ext/skills/src/providers/host.rs create mode 100644 codex-rs/ext/skills/src/providers/mod.rs create mode 100644 codex-rs/ext/skills/src/providers/remote.rs create mode 100644 codex-rs/ext/skills/src/state.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 33392db93..94dac4569 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3715,6 +3715,15 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "codex-skills-extension" +version = "0.0.0" +dependencies = [ + "async-trait", + "codex-core", + "codex-extension-api", +] + [[package]] name = "codex-state" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index cc20f8a3a..482951fb9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -48,6 +48,7 @@ members = [ "ext/guardian", "ext/image-generation", "ext/memories", + "ext/skills", "ext/web-search", "external-agent-migration", "external-agent-sessions", diff --git a/codex-rs/ext/skills/BUILD.bazel b/codex-rs/ext/skills/BUILD.bazel new file mode 100644 index 000000000..81410ca1f --- /dev/null +++ b/codex-rs/ext/skills/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "skills", + crate_name = "codex_skills_extension", +) diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml new file mode 100644 index 000000000..55671df6c --- /dev/null +++ b/codex-rs/ext/skills/Cargo.toml @@ -0,0 +1,19 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-skills-extension" +version.workspace = true + +[lib] +name = "codex_skills_extension" +path = "src/lib.rs" +test = false +doctest = false + +[lints] +workspace = true + +[dependencies] +async-trait = { workspace = true } +codex-core = { workspace = true } +codex-extension-api = { workspace = true } diff --git a/codex-rs/ext/skills/src/catalog.rs b/codex-rs/ext/skills/src/catalog.rs new file mode 100644 index 000000000..86ae327d4 --- /dev/null +++ b/codex-rs/ext/skills/src/catalog.rs @@ -0,0 +1,86 @@ +/// Source authority that owns a skill package and must be used to read it. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum SkillSourceKind { + Host, + Executor, + Remote, +} + +/// Opaque authority identity for list/read routing. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SkillAuthority { + pub kind: SkillSourceKind, + pub id: String, +} + +impl SkillAuthority { + pub fn new(kind: SkillSourceKind, id: impl Into) -> Self { + Self { + kind, + id: id.into(), + } + } +} + +/// Opaque package id. Callers should not parse local paths out of this value. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SkillPackageId(pub String); + +/// Opaque resource id inside a skill package. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SkillResourceId(pub String); + +/// Metadata shown in the always-visible skills catalog. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillCatalogEntry { + pub id: SkillPackageId, + pub authority: SkillAuthority, + pub name: String, + pub description: String, + pub entrypoint: SkillResourceId, +} + +/// Merged catalog for one turn. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SkillCatalog { + pub entries: Vec, + pub warnings: Vec, +} + +impl SkillCatalog { + pub fn extend(&mut self, other: SkillCatalog) { + // TODO(skills-extension): dedupe by authority-bound id first, then + // apply name precedence/conflict rules for user-facing mention + // resolution. Names are not stable identities. + self.entries.extend(other.entries); + self.warnings.extend(other.warnings); + } +} + +/// Contents returned after resolving a skill resource through its owner. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillReadResult { + pub resource: SkillResourceId, + pub contents: String, +} + +/// Search results for a package whose files are not readable through ordinary +/// executor filesystem access. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SkillSearchResult { + pub matches: Vec, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillSearchMatch { + pub resource: SkillResourceId, + pub title: String, + pub snippet: String, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillProviderError { + pub message: String, +} + +pub type SkillProviderResult = Result; diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs new file mode 100644 index 000000000..bf91b60de --- /dev/null +++ b/codex-rs/ext/skills/src/extension.rs @@ -0,0 +1,140 @@ +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; + +use codex_core::config::Config; +use codex_extension_api::ConfigContributor; +use codex_extension_api::ContextContributor; +use codex_extension_api::ExtensionData; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::PromptFragment; +use codex_extension_api::ThreadLifecycleContributor; +use codex_extension_api::ThreadStartInput; +use codex_extension_api::TurnLifecycleContributor; +use codex_extension_api::TurnStartInput; + +use crate::provider::SkillListQuery; +use crate::providers::SkillProviders; +use crate::state::SkillsExtensionConfig; +use crate::state::SkillsTurnState; + +#[derive(Clone, Debug, Default)] +struct SkillsExtension { + providers: SkillProviders, +} + +#[async_trait::async_trait] +impl ThreadLifecycleContributor for SkillsExtension { + async fn on_thread_start(&self, input: ThreadStartInput<'_, Config>) { + // TODO(skills-extension): this is only the thread-level config snapshot. + // Skills are loaded per turn today because cwd, plugin roots, config + // layers, and the primary environment filesystem can change between + // turns. The real migration needs a turn-preparation hook before model + // input construction, not just thread startup. + input + .thread_store + .insert(SkillsExtensionConfig::from_config(input.config)); + } +} + +impl ConfigContributor for SkillsExtension { + fn on_config_changed( + &self, + _session_store: &ExtensionData, + thread_store: &ExtensionData, + _previous_config: &Config, + new_config: &Config, + ) { + // TODO(skills-extension): update any cached/listing state that depends + // on skill config overrides, bundled skills, or include_instructions. + thread_store.insert(SkillsExtensionConfig::from_config(new_config)); + } +} + +impl ContextContributor for SkillsExtension { + fn contribute<'a>( + &'a self, + _session_store: &'a ExtensionData, + thread_store: &'a ExtensionData, + ) -> Pin> + Send + 'a>> { + Box::pin(async move { + let Some(config) = thread_store.get::() else { + return Vec::new(); + }; + if !config.include_instructions { + return Vec::new(); + } + + // TODO(skills-extension): render the available-skills developer + // block from the merged per-turn SkillCatalog. This should + // preserve the existing bounded metadata budget, root aliasing, + // warning behavior, and telemetry side effects. + // + // TODO(skills-extension): avoid using raw PromptFragment strings + // for final skills context if the extension API grows typed + // contextual fragments. Existing skill blocks are typed so resume + // and history filtering can recognize them reliably. + // + // TODO(skills-extension): ContextContributor currently cannot see + // the turn_store, so it cannot read the per-turn catalog seeded by + // the turn provider path below. This is the main extension-api gap + // to close before skills can move out of codex-core. + Vec::new() + }) + } +} + +#[async_trait::async_trait] +impl TurnLifecycleContributor for SkillsExtension { + async fn on_turn_start(&self, input: TurnStartInput<'_>) { + // TODO(skills-extension): replace this lifecycle callback with a real + // turn-input contributor in codex-extension-api. This placeholder only + // demonstrates where provider aggregation belongs; it cannot resolve + // real skills because this hook does not receive cwd, executor + // selections, effective plugins/materialized plugin skill roots, + // connector slug counts, user input, cancellation, analytics, or a + // response-item output channel. + let query = SkillListQuery::placeholder_for_turn(input.turn_id); + let catalog = self + .providers + .list_for_turn(query) + .await + .unwrap_or_default(); + + input.turn_store.insert(SkillsTurnState { + catalog, + entrypoints_injected: false, + }); + + // TODO(skills-extension): after catalog resolution, collect explicit + // skill mentions from structured UserInput and text mentions. + // + // TODO(skills-extension): inject selected entrypoints as typed + // contextual user fragments, preserving ... history + // recognition and bounded body size limits. + // + // TODO(skills-extension): move explicit $skill mention resolution, + // SKILL.md reads, skill body injection, and MCP dependency prompting + // out of codex-core's turn assembly once that hook exists. + } +} + +/// Installs the skills extension contributor sketch. +/// +/// TODO(skills-extension): pass host capabilities here rather than letting the +/// extension depend on Session. The final extension needs capability objects for +/// loading skill roots, emitting warnings, tracking analytics, prompting for MCP +/// dependency install, refreshing MCP servers, and serving app-server catalog +/// requests. +/// +/// TODO(skills-extension): plugin handling should stay outside the runtime +/// skills model. Plugins are bundle/install units; once installed or refreshed, +/// their skill descriptors/roots should be handed to this extension just like +/// any other host-owned skill source. +pub fn install(registry: &mut ExtensionRegistryBuilder) { + let extension = Arc::new(SkillsExtension::default()); + registry.thread_lifecycle_contributor(extension.clone()); + registry.config_contributor(extension.clone()); + registry.prompt_contributor(extension.clone()); + registry.turn_lifecycle_contributor(extension); +} diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs new file mode 100644 index 000000000..6e8a0700a --- /dev/null +++ b/codex-rs/ext/skills/src/lib.rs @@ -0,0 +1,7 @@ +pub mod catalog; +mod extension; +pub mod provider; +mod providers; +mod state; + +pub use extension::install; diff --git a/codex-rs/ext/skills/src/provider.rs b/codex-rs/ext/skills/src/provider.rs new file mode 100644 index 000000000..730adb51b --- /dev/null +++ b/codex-rs/ext/skills/src/provider.rs @@ -0,0 +1,63 @@ +use std::future::Future; + +use crate::catalog::SkillAuthority; +use crate::catalog::SkillCatalog; +use crate::catalog::SkillPackageId; +use crate::catalog::SkillProviderResult; +use crate::catalog::SkillReadResult; +use crate::catalog::SkillResourceId; +use crate::catalog::SkillSearchResult; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillListQuery { + pub turn_id: String, + pub executor_authorities: Vec, + pub include_host_skills: bool, + pub include_remote_skills: bool, +} + +impl SkillListQuery { + pub(crate) fn placeholder_for_turn(turn_id: &str) -> Self { + Self { + turn_id: turn_id.to_string(), + executor_authorities: Vec::new(), + include_host_skills: true, + include_remote_skills: true, + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillReadRequest { + pub authority: SkillAuthority, + pub resource: SkillResourceId, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SkillSearchRequest { + pub authority: SkillAuthority, + pub package: SkillPackageId, + pub query: String, +} + +/// Source-specific skill catalog and resource access. +/// +/// Implementations must preserve authority boundaries: a resource listed by a +/// provider must be read or searched through the same provider/authority rather +/// than converted into an ambient local path. +pub trait SkillProvider: Send + Sync { + fn list( + &self, + query: SkillListQuery, + ) -> impl Future> + Send; + + fn read( + &self, + request: SkillReadRequest, + ) -> impl Future> + Send; + + fn search( + &self, + request: SkillSearchRequest, + ) -> impl Future> + Send; +} diff --git a/codex-rs/ext/skills/src/providers/executor.rs b/codex-rs/ext/skills/src/providers/executor.rs new file mode 100644 index 000000000..f9f399eeb --- /dev/null +++ b/codex-rs/ext/skills/src/providers/executor.rs @@ -0,0 +1,56 @@ +use std::future; + +use crate::catalog::SkillCatalog; +use crate::catalog::SkillProviderResult; +use crate::catalog::SkillReadResult; +use crate::catalog::SkillSearchResult; +use crate::provider::SkillListQuery; +use crate::provider::SkillProvider; +use crate::provider::SkillReadRequest; +use crate::provider::SkillSearchRequest; + +#[derive(Clone, Debug, Default)] +pub(crate) struct ExecutorSkillProvider; + +impl SkillProvider for ExecutorSkillProvider { + fn list( + &self, + _query: SkillListQuery, + ) -> impl Future> + Send { + future::ready(Ok(SkillCatalog::default())) + // TODO(skills-extension): list repo/workspace skills from each + // executor authority selected for the turn. + // + // TODO(skills-extension): if the executor exposes filesystem reads, + // preserve the existing SKILL.md discovery semantics. If CCA/no-FS + // applies, query an executor catalog/read capability instead. + // + // TODO(skills-extension): include the executor/environment id in skill + // identity so two executors with the same path/name do not collide. + } + + fn read( + &self, + request: SkillReadRequest, + ) -> impl Future> + Send { + future::ready(Err(crate::catalog::SkillProviderError { + message: format!( + "executor skill resource `{}` is not implemented", + request.resource.0 + ), + })) + // TODO(skills-extension): route reads back to the executor authority + // that listed the resource. Do not mint local paths from remote or + // non-filesystem executor resources. + } + + fn search( + &self, + _request: SkillSearchRequest, + ) -> impl Future> + Send { + future::ready(Ok(SkillSearchResult::default())) + // TODO(skills-extension): support search for executor skills only when + // the executor offers a catalog/search API. For ordinary filesystem + // executors, the model can keep using regular file reads/search tools. + } +} diff --git a/codex-rs/ext/skills/src/providers/host.rs b/codex-rs/ext/skills/src/providers/host.rs new file mode 100644 index 000000000..fc4317f85 --- /dev/null +++ b/codex-rs/ext/skills/src/providers/host.rs @@ -0,0 +1,65 @@ +use std::future; + +use crate::catalog::SkillCatalog; +use crate::catalog::SkillProviderResult; +use crate::catalog::SkillReadResult; +use crate::catalog::SkillSearchResult; +use crate::provider::SkillListQuery; +use crate::provider::SkillProvider; +use crate::provider::SkillReadRequest; +use crate::provider::SkillSearchRequest; + +#[derive(Clone, Debug, Default)] +pub(crate) struct HostSkillProvider; + +impl SkillProvider for HostSkillProvider { + fn list( + &self, + _query: SkillListQuery, + ) -> impl Future> + Send { + future::ready(Ok(SkillCatalog::default())) + // TODO(skills-extension): list bundled/system/user/plugin-installed + // skills owned by the Codex host. This is the source for skills that + // are not tied to a particular executor authority. + // + // TODO(skills-extension): plugins should be treated as packaging and + // installation only. After a plugin is downloaded, cached, refreshed, + // or installed, its skill roots/descriptors should enter this provider + // and then use the normal skills catalog/read/injection code. + // + // TODO(skills-extension): remote skills that are materialized locally + // by plugin install or explicit download should also hand off here + // rather than remain remote-provider entries at runtime. + // + // TODO(skills-extension): keep current bundled system skill install or + // replace it with embedded host assets so CCA/no-FS hosts do not depend + // on local writable skill cache directories. + } + + fn read( + &self, + request: SkillReadRequest, + ) -> impl Future> + Send { + future::ready(Err(crate::catalog::SkillProviderError { + message: format!( + "host skill resource `{}` is not implemented", + request.resource.0 + ), + })) + // TODO(skills-extension): read host-owned entrypoints and supporting + // resources by opaque id, not by assuming a local filesystem path. + // + // TODO(skills-extension): for plugin-installed skills, route reads + // through the materialized plugin cache/root that produced the catalog + // entry, while keeping the public id opaque and authority-bound. + } + + fn search( + &self, + _request: SkillSearchRequest, + ) -> impl Future> + Send { + future::ready(Ok(SkillSearchResult::default())) + // TODO(skills-extension): decide whether host skills need search, or + // whether direct read by opaque resource id is enough. + } +} diff --git a/codex-rs/ext/skills/src/providers/mod.rs b/codex-rs/ext/skills/src/providers/mod.rs new file mode 100644 index 000000000..fcf278848 --- /dev/null +++ b/codex-rs/ext/skills/src/providers/mod.rs @@ -0,0 +1,50 @@ +mod executor; +mod host; +mod remote; + +use crate::catalog::SkillCatalog; +use crate::catalog::SkillProviderResult; +use crate::provider::SkillListQuery; +use crate::provider::SkillProvider; + +use executor::ExecutorSkillProvider; +use host::HostSkillProvider; +use remote::RemoteSkillProvider; + +#[derive(Clone, Debug, Default)] +pub(crate) struct SkillProviders { + host: HostSkillProvider, + executor: ExecutorSkillProvider, + remote: RemoteSkillProvider, +} + +impl SkillProviders { + pub(crate) async fn list_for_turn( + &self, + query: SkillListQuery, + ) -> SkillProviderResult { + let mut catalog = SkillCatalog::default(); + + if query.include_host_skills { + catalog.extend(self.host.list(query.clone()).await?); + } + + if !query.executor_authorities.is_empty() { + catalog.extend(self.executor.list(query.clone()).await?); + } + + if query.include_remote_skills { + catalog.extend(self.remote.list(query).await?); + } + + // TODO(skills-extension): apply final merged-catalog policy here: + // source precedence, duplicate name handling, disabled-skill rules, + // product/session-source filtering, and telemetry for omitted entries. + // + // TODO(skills-extension): treat plugin-installed skills as ordinary + // host catalog entries by this point. Plugin identity may remain useful + // for display, auth, and uninstall flows, but mention resolution and + // entrypoint injection should not special-case plugin packaging. + Ok(catalog) + } +} diff --git a/codex-rs/ext/skills/src/providers/remote.rs b/codex-rs/ext/skills/src/providers/remote.rs new file mode 100644 index 000000000..22dd3368a --- /dev/null +++ b/codex-rs/ext/skills/src/providers/remote.rs @@ -0,0 +1,58 @@ +use std::future; + +use crate::catalog::SkillCatalog; +use crate::catalog::SkillProviderResult; +use crate::catalog::SkillReadResult; +use crate::catalog::SkillSearchResult; +use crate::provider::SkillListQuery; +use crate::provider::SkillProvider; +use crate::provider::SkillReadRequest; +use crate::provider::SkillSearchRequest; + +#[derive(Clone, Debug, Default)] +pub(crate) struct RemoteSkillProvider; + +impl SkillProvider for RemoteSkillProvider { + fn list( + &self, + _query: SkillListQuery, + ) -> impl Future> + Send { + future::ready(Ok(SkillCatalog::default())) + // TODO(skills-extension): list org/account/backend skills from a + // remote catalog only when they are not installed/materialized into the + // host. These skills should use opaque ids and backend authority, not + // paths. + // + // TODO(skills-extension): if a remote skill is downloaded or installed + // as part of a plugin-like bundle, hand it to HostSkillProvider for + // runtime listing/read instead of keeping a separate remote runtime + // path. + // + // TODO(skills-extension): decide how org policy and local enable/disable + // rules combine when the backend supplies a managed skill catalog. + } + + fn read( + &self, + request: SkillReadRequest, + ) -> impl Future> + Send { + future::ready(Err(crate::catalog::SkillProviderError { + message: format!( + "remote skill resource `{}` is not implemented", + request.resource.0 + ), + })) + // TODO(skills-extension): read remote skill entrypoints and supporting + // files through authenticated backend APIs. + } + + fn search( + &self, + _request: SkillSearchRequest, + ) -> impl Future> + Send { + future::ready(Ok(SkillSearchResult::default())) + // TODO(skills-extension): expose model-facing skills/search or resource + // APIs for large remote packages so the model can progressively + // disclose supporting files without filesystem access. + } +} diff --git a/codex-rs/ext/skills/src/state.rs b/codex-rs/ext/skills/src/state.rs new file mode 100644 index 000000000..d6c355ed3 --- /dev/null +++ b/codex-rs/ext/skills/src/state.rs @@ -0,0 +1,24 @@ +use codex_core::config::Config; + +use crate::catalog::SkillCatalog; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct SkillsExtensionConfig { + pub(crate) include_instructions: bool, + pub(crate) bundled_skills_enabled: bool, +} + +impl SkillsExtensionConfig { + pub(crate) fn from_config(config: &Config) -> Self { + Self { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + } + } +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(crate) struct SkillsTurnState { + pub(crate) catalog: SkillCatalog, + pub(crate) entrypoints_injected: bool, +}