From 96d2d2f68c07ef01650db1cba01071fbb2d98218 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 3 Jun 2026 16:24:16 +0200 Subject: [PATCH] Implement v1 skills extension prompt injection (#26167) ## Why The skills extension needs a real turn-time path before host, executor, or remote skills can be routed through it. The previous code was mostly a placeholder catalog/provider sketch, so there was no bounded available-skills fragment, no source-owned `SKILL.md` read, and no place for warnings or per-turn selection state to live. This PR makes `ext/skills` the authority-preserving flow for listing candidate skills and injecting only explicitly selected main prompts, without adding more of that logic to `codex-core`. ## What changed - Expands catalog entries with `main_prompt`, display path, short description, dependency metadata, enabled/prompt visibility flags, and authority/package-aware read requests. - Replaces the placeholder `providers/*` modules with `SkillProviderSource` and `SkillProviders`, routing list/read/search calls by source kind and surfacing provider failures as warnings. - Adds bounded available-skills rendering and `SKILL.md` main-prompt truncation before the fragments enter model context. - Resolves explicit skill selections from structured `UserInput::Skill`, skill-file mentions, `skill://...` paths, and plain `$skill` text mentions, then reads selected prompts through their owning provider. - Stores mutable per-thread skills config and per-turn catalog/selection/warning state. - Adds `install_with_providers` so tests and future host wiring can supply concrete providers. ## Testing - Not run locally. - Added `codex-rs/ext/skills/tests/skills_extension.rs` coverage for available-catalog injection, selected prompt injection through the owning provider, and prompt-hidden skills that remain invokable. --- codex-rs/Cargo.lock | 4 + codex-rs/ext/skills/Cargo.toml | 6 + codex-rs/ext/skills/src/catalog.rs | 126 +++++++- codex-rs/ext/skills/src/extension.rs | 208 +++++++----- codex-rs/ext/skills/src/lib.rs | 7 +- codex-rs/ext/skills/src/provider.rs | 21 +- 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/render.rs | 90 ++++++ codex-rs/ext/skills/src/selection.rs | 129 ++++++++ codex-rs/ext/skills/src/sources.rs | 183 +++++++++++ codex-rs/ext/skills/src/state.rs | 33 +- codex-rs/ext/skills/tests/skills_extension.rs | 300 ++++++++++++++++++ 15 files changed, 1001 insertions(+), 335 deletions(-) delete mode 100644 codex-rs/ext/skills/src/providers/executor.rs delete mode 100644 codex-rs/ext/skills/src/providers/host.rs delete mode 100644 codex-rs/ext/skills/src/providers/mod.rs delete mode 100644 codex-rs/ext/skills/src/providers/remote.rs create mode 100644 codex-rs/ext/skills/src/render.rs create mode 100644 codex-rs/ext/skills/src/selection.rs create mode 100644 codex-rs/ext/skills/src/sources.rs create mode 100644 codex-rs/ext/skills/tests/skills_extension.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 0b599b419..2183d91b6 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3733,7 +3733,11 @@ version = "0.0.0" dependencies = [ "async-trait", "codex-core", + "codex-core-skills", "codex-extension-api", + "codex-protocol", + "pretty_assertions", + "tokio", ] [[package]] diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml index 55671df6c..29767d58f 100644 --- a/codex-rs/ext/skills/Cargo.toml +++ b/codex-rs/ext/skills/Cargo.toml @@ -16,4 +16,10 @@ workspace = true [dependencies] async-trait = { workspace = true } codex-core = { workspace = true } +codex-core-skills = { workspace = true } codex-extension-api = { workspace = true } +codex-protocol = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/codex-rs/ext/skills/src/catalog.rs b/codex-rs/ext/skills/src/catalog.rs index 86ae327d4..a09a81ff7 100644 --- a/codex-rs/ext/skills/src/catalog.rs +++ b/codex-rs/ext/skills/src/catalog.rs @@ -1,9 +1,39 @@ +use codex_core_skills::model::SkillDependencies; + /// Source authority that owns a skill package and must be used to read it. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum SkillSourceKind { + /// Codex-hosted skills, including bundled, user, repo, plugin-installed, + /// and downloaded/materialized remote skills. Host, + /// Skills owned by an execution environment. Executor, + /// Skills read through an authenticated remote catalog/API. Remote, + /// Extension-private source kind for future providers that do not fit an + /// existing transport category. + Custom(String), +} + +impl SkillSourceKind { + pub fn custom(kind: impl Into) -> Self { + Self::Custom(kind.into()) + } + + fn as_str(&self) -> &str { + match self { + Self::Host => "host", + Self::Executor => "executor", + Self::Remote => "remote", + Self::Custom(kind) => kind, + } + } +} + +impl std::fmt::Display for SkillSourceKind { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.as_str().fmt(formatter) + } } /// Opaque authority identity for list/read routing. @@ -37,7 +67,66 @@ pub struct SkillCatalogEntry { pub authority: SkillAuthority, pub name: String, pub description: String, - pub entrypoint: SkillResourceId, + pub short_description: Option, + pub main_prompt: SkillResourceId, + pub display_path: Option, + pub dependencies: Option, + pub enabled: bool, + pub prompt_visible: bool, +} + +impl SkillCatalogEntry { + pub fn new( + id: SkillPackageId, + authority: SkillAuthority, + name: impl Into, + description: impl Into, + main_prompt: SkillResourceId, + ) -> Self { + Self { + id, + authority, + name: name.into(), + description: description.into(), + short_description: None, + main_prompt, + display_path: None, + dependencies: None, + enabled: true, + prompt_visible: true, + } + } + + pub fn with_short_description(mut self, short_description: Option) -> Self { + self.short_description = short_description; + self + } + + pub fn with_display_path(mut self, display_path: impl Into) -> Self { + self.display_path = Some(display_path.into()); + self + } + + pub fn with_dependencies(mut self, dependencies: Option) -> Self { + self.dependencies = dependencies; + self + } + + pub fn disabled(mut self) -> Self { + self.enabled = false; + self + } + + pub fn hidden_from_prompt(mut self) -> Self { + self.prompt_visible = false; + self + } + + pub(crate) fn rendered_path(&self) -> &str { + self.display_path + .as_deref() + .unwrap_or(self.main_prompt.0.as_str()) + } } /// Merged catalog for one turn. @@ -49,12 +138,23 @@ pub struct SkillCatalog { 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); + for entry in other.entries { + self.push_entry(entry); + } self.warnings.extend(other.warnings); } + + pub fn push_entry(&mut self, entry: SkillCatalogEntry) { + if self + .entries + .iter() + .any(|existing| existing.authority == entry.authority && existing.id == entry.id) + { + return; + } + + self.entries.push(entry); + } } /// Contents returned after resolving a skill resource through its owner. @@ -83,4 +183,20 @@ pub struct SkillProviderError { pub message: String, } +impl SkillProviderError { + pub fn new(message: impl Into) -> Self { + Self { + message: message.into(), + } + } +} + +impl std::fmt::Display for SkillProviderError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.message.fmt(formatter) + } +} + +impl std::error::Error for SkillProviderError {} + pub type SkillProviderResult = Result; diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index adbaa8ffe..85ff513ac 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -1,42 +1,49 @@ -use std::future::Future; -use std::pin::Pin; use std::sync::Arc; use codex_core::config::Config; +use codex_core_skills::SkillInstructions; +use codex_core_skills::injection::SkillInjection; use codex_extension_api::ConfigContributor; -use codex_extension_api::ContextContributor; use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; +use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistryBuilder; -use codex_extension_api::PromptFragment; use codex_extension_api::ThreadLifecycleContributor; use codex_extension_api::ThreadStartInput; use codex_extension_api::TurnInputContext; use codex_extension_api::TurnInputContributor; +use codex_protocol::protocol::Event; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::WarningEvent; use crate::catalog::SkillAuthority; +use crate::catalog::SkillCatalogEntry; +use crate::catalog::SkillReadResult; use crate::catalog::SkillSourceKind; use crate::provider::SkillListQuery; -use crate::providers::SkillProviders; +use crate::provider::SkillReadRequest; +use crate::render::available_skills_fragment; +use crate::render::truncate_main_prompt_contents; +use crate::selection::collect_explicit_skill_mentions; +use crate::sources::SkillProviders; use crate::state::SkillsExtensionConfig; +use crate::state::SkillsThreadState; use crate::state::SkillsTurnState; -#[derive(Clone, Debug, Default)] +#[derive(Clone)] struct SkillsExtension { providers: SkillProviders, + event_sink: Arc, } #[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 TurnInputContributor below owns per-turn catalog - // resolution. input .thread_store - .insert(SkillsExtensionConfig::from_config(input.config)); + .insert(SkillsThreadState::new(SkillsExtensionConfig::from_config( + input.config, + ))); } } @@ -48,36 +55,12 @@ impl ConfigContributor for SkillsExtension { _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 only if the final model needs thread-level guidance that - // does not depend on the per-turn SkillCatalog. - // - // 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. - Vec::new() - }) + let next_config = SkillsExtensionConfig::from_config(new_config); + if let Some(state) = thread_store.get::() { + state.set_config(next_config); + } else { + thread_store.insert(SkillsThreadState::new(next_config)); + } } } @@ -87,67 +70,118 @@ impl TurnInputContributor for SkillsExtension { &self, input: TurnInputContext, _session_store: &ExtensionData, - _thread_store: &ExtensionData, + thread_store: &ExtensionData, turn_store: &ExtensionData, ) -> Vec> { - let executor_authorities = input - .environments - .iter() - .map(|environment| { - SkillAuthority::new( - SkillSourceKind::Executor, - environment.environment_id.clone(), - ) - }) - .collect(); + let Some(thread_state) = thread_store.get::() else { + return Vec::new(); + }; + + let config = thread_state.config(); let query = SkillListQuery { - turn_id: input.turn_id, - executor_authorities, + turn_id: input.turn_id.clone(), + executor_authorities: input + .environments + .iter() + .map(|environment| { + SkillAuthority::new( + SkillSourceKind::Executor, + environment.environment_id.clone(), + ) + }) + .collect(), include_host_skills: true, + include_bundled_skills: config.bundled_skills_enabled, include_remote_skills: true, }; - let catalog = self - .providers - .list_for_turn(query) - .await - .unwrap_or_default(); + let catalog = self.providers.list_for_turn(query).await; + for warning in &catalog.warnings { + self.emit_warning(&input.turn_id, warning.clone()); + } + + let selected_entries = collect_explicit_skill_mentions(&input.user_input, &catalog); + let mut fragments: Vec> = Vec::new(); + if config.include_instructions + && let Some(fragment) = available_skills_fragment(&catalog) + { + fragments.push(Box::new(fragment)); + } + + let mut warnings = catalog.warnings.clone(); + let mut main_prompts_injected = false; + for entry in &selected_entries { + match self.read_main_prompt(entry).await { + Ok(read_result) => { + let (contents, truncated) = + truncate_main_prompt_contents(read_result.contents.as_str()); + if truncated { + let warning = format!( + "Skill `{}` exceeded the main prompt context limit and was truncated.", + entry.name + ); + self.emit_warning(&input.turn_id, warning.clone()); + warnings.push(warning); + } + let injection = SkillInjection { + name: entry.name.clone(), + path: entry.rendered_path().to_string(), + contents, + }; + fragments.push(Box::new(SkillInstructions::from(&injection))); + main_prompts_injected = true; + } + Err(message) => { + let warning = format!("Failed to load skill `{}`: {message}", entry.name); + self.emit_warning(&input.turn_id, warning.clone()); + warnings.push(warning); + } + } + } turn_store.insert(SkillsTurnState { catalog, - entrypoints_injected: false, + selected_entries, + warnings, + main_prompts_injected, }); - // 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 provider implementations - // are ready to preserve behavior. - Vec::new() + fragments + } +} + +impl SkillsExtension { + async fn read_main_prompt(&self, entry: &SkillCatalogEntry) -> Result { + self.providers + .read(SkillReadRequest { + authority: entry.authority.clone(), + package: entry.id.clone(), + resource: entry.main_prompt.clone(), + }) + .await + .map_err(|err| err.message) + } + + fn emit_warning(&self, turn_id: &str, message: String) { + self.event_sink.emit(Event { + id: turn_id.to_string(), + msg: EventMsg::Warning(WarningEvent { message }), + }); } } -/// 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()); + install_with_providers(registry, SkillProviders::default()); +} + +pub fn install_with_providers( + registry: &mut ExtensionRegistryBuilder, + providers: SkillProviders, +) { + let extension = Arc::new(SkillsExtension { + providers, + event_sink: registry.event_sink(), + }); registry.thread_lifecycle_contributor(extension.clone()); registry.config_contributor(extension.clone()); - registry.prompt_contributor(extension.clone()); registry.turn_input_contributor(extension); } diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 6e8a0700a..b5e224d49 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -1,7 +1,12 @@ pub mod catalog; mod extension; pub mod provider; -mod providers; +mod render; +mod selection; +mod sources; mod state; pub use extension::install; +pub use extension::install_with_providers; +pub use sources::SkillProviderSource; +pub use sources::SkillProviders; diff --git a/codex-rs/ext/skills/src/provider.rs b/codex-rs/ext/skills/src/provider.rs index b6f240752..f9c1b41fe 100644 --- a/codex-rs/ext/skills/src/provider.rs +++ b/codex-rs/ext/skills/src/provider.rs @@ -1,4 +1,5 @@ use std::future::Future; +use std::pin::Pin; use crate::catalog::SkillAuthority; use crate::catalog::SkillCatalog; @@ -13,12 +14,14 @@ pub struct SkillListQuery { pub turn_id: String, pub executor_authorities: Vec, pub include_host_skills: bool, + pub include_bundled_skills: bool, pub include_remote_skills: bool, } #[derive(Clone, Debug, PartialEq, Eq)] pub struct SkillReadRequest { pub authority: SkillAuthority, + pub package: SkillPackageId, pub resource: SkillResourceId, } @@ -29,24 +32,18 @@ pub struct SkillSearchRequest { pub query: String, } +pub type SkillProviderFuture<'a, T> = + Pin> + Send + 'a>>; + /// 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 list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog>; - fn read( - &self, - request: SkillReadRequest, - ) -> impl Future> + Send; + fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult>; - fn search( - &self, - request: SkillSearchRequest, - ) -> impl Future> + Send; + fn search(&self, request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult>; } diff --git a/codex-rs/ext/skills/src/providers/executor.rs b/codex-rs/ext/skills/src/providers/executor.rs deleted file mode 100644 index f9f399eeb..000000000 --- a/codex-rs/ext/skills/src/providers/executor.rs +++ /dev/null @@ -1,56 +0,0 @@ -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 deleted file mode 100644 index fc4317f85..000000000 --- a/codex-rs/ext/skills/src/providers/host.rs +++ /dev/null @@ -1,65 +0,0 @@ -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 deleted file mode 100644 index fcf278848..000000000 --- a/codex-rs/ext/skills/src/providers/mod.rs +++ /dev/null @@ -1,50 +0,0 @@ -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 deleted file mode 100644 index 22dd3368a..000000000 --- a/codex-rs/ext/skills/src/providers/remote.rs +++ /dev/null @@ -1,58 +0,0 @@ -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/render.rs b/codex-rs/ext/skills/src/render.rs new file mode 100644 index 000000000..157ae4bff --- /dev/null +++ b/codex-rs/ext/skills/src/render.rs @@ -0,0 +1,90 @@ +use codex_core_skills::render_available_skills_body; +use codex_extension_api::ContextualUserFragment; +use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; +use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; + +use crate::catalog::SkillCatalog; + +const MAX_AVAILABLE_SKILLS_CHARS: usize = 8_000; +const MAX_MAIN_PROMPT_CHARS: usize = 40_000; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct AvailableSkillsFragment { + body: String, +} + +impl ContextualUserFragment for AvailableSkillsFragment { + fn role(&self) -> &'static str { + "developer" + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn body(&self) -> String { + self.body.clone() + } + + fn type_markers() -> (&'static str, &'static str) { + (SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG) + } +} + +pub(crate) fn available_skills_fragment(catalog: &SkillCatalog) -> Option { + let mut total_chars = 0usize; + let mut omitted = 0usize; + let mut skill_lines = Vec::new(); + + for entry in catalog + .entries + .iter() + .filter(|entry| entry.enabled && entry.prompt_visible) + { + let description = entry + .short_description + .as_deref() + .unwrap_or(entry.description.as_str()); + let line = render_skill_line(entry.name.as_str(), description, entry.rendered_path()); + let next_chars = total_chars.saturating_add(line.chars().count()); + if next_chars > MAX_AVAILABLE_SKILLS_CHARS { + omitted = omitted.saturating_add(1); + continue; + } + total_chars = next_chars; + skill_lines.push(line); + } + + if skill_lines.is_empty() { + return None; + } + if omitted > 0 { + let skill_word = if omitted == 1 { "skill" } else { "skills" }; + skill_lines.push(format!( + "- {omitted} additional {skill_word} omitted from this bounded skills list." + )); + } + + Some(AvailableSkillsFragment { + body: render_available_skills_body(&[], &skill_lines), + }) +} + +fn render_skill_line(name: &str, description: &str, path: &str) -> String { + if description.is_empty() { + format!("- {name}: (file: {path})") + } else { + format!("- {name}: {description} (file: {path})") + } +} + +pub(crate) fn truncate_main_prompt_contents(contents: &str) -> (String, bool) { + let mut chars = 0usize; + for (index, _) in contents.char_indices() { + if chars == MAX_MAIN_PROMPT_CHARS { + return (contents[..index].to_string(), true); + } + chars = chars.saturating_add(1); + } + (contents.to_string(), false) +} diff --git a/codex-rs/ext/skills/src/selection.rs b/codex-rs/ext/skills/src/selection.rs new file mode 100644 index 000000000..c4405142e --- /dev/null +++ b/codex-rs/ext/skills/src/selection.rs @@ -0,0 +1,129 @@ +use std::collections::HashSet; + +use codex_core_skills::injection::extract_tool_mentions; +use codex_protocol::user_input::UserInput; + +use crate::catalog::SkillAuthority; +use crate::catalog::SkillCatalog; +use crate::catalog::SkillCatalogEntry; +use crate::catalog::SkillPackageId; + +const SKILL_PATH_PREFIX: &str = "skill://"; + +pub(crate) fn collect_explicit_skill_mentions( + inputs: &[UserInput], + catalog: &SkillCatalog, +) -> Vec { + let mut selected = Vec::new(); + let mut seen = HashSet::new(); + let mut blocked_plain_names = HashSet::new(); + + for input in inputs { + match input { + UserInput::Skill { name, path } => { + blocked_plain_names.insert(name.clone()); + select_by_path(catalog, &path.to_string_lossy(), &mut seen, &mut selected); + } + UserInput::Mention { name, path } if path_is_skill(path) => { + blocked_plain_names.insert(name.clone()); + select_by_path(catalog, path, &mut seen, &mut selected); + } + UserInput::Text { .. } | UserInput::Image { .. } | UserInput::LocalImage { .. } => {} + UserInput::Mention { .. } => {} + _ => {} + } + } + + for input in inputs { + let UserInput::Text { text, .. } = input else { + continue; + }; + + let mentions = extract_tool_mentions(text); + for path in mentions.paths() { + if path_is_skill(path) { + select_by_path( + catalog, + normalize_skill_path(path), + &mut seen, + &mut selected, + ); + } + } + for name in mentions.plain_names() { + if blocked_plain_names.contains(name) { + continue; + } + if let Some(entry) = catalog + .entries + .iter() + .find(|entry| entry.enabled && entry.name == name) + { + push_selected(entry, &mut seen, &mut selected); + } + } + } + + selected +} + +fn select_by_path( + catalog: &SkillCatalog, + path: &str, + seen: &mut HashSet, + selected: &mut Vec, +) { + let normalized_path = normalize_skill_path(path); + for entry in catalog.entries.iter().filter(|entry| entry.enabled) { + if entry_matches_path(entry, normalized_path) { + push_selected(entry, seen, selected); + } + } +} + +fn push_selected( + entry: &SkillCatalogEntry, + seen: &mut HashSet, + selected: &mut Vec, +) { + let key = SkillCatalogEntryKey::from(entry); + if seen.insert(key) { + selected.push(entry.clone()); + } +} + +fn entry_matches_path(entry: &SkillCatalogEntry, path: &str) -> bool { + entry.main_prompt.0 == path + || entry.id.0 == path + || entry + .display_path + .as_deref() + .is_some_and(|display_path| display_path == path) +} + +fn path_is_skill(path: &str) -> bool { + path.starts_with(SKILL_PATH_PREFIX) + || path + .rsplit(['/', '\\']) + .next() + .is_some_and(|file_name| file_name.eq_ignore_ascii_case("SKILL.md")) +} + +fn normalize_skill_path(path: &str) -> &str { + path.strip_prefix(SKILL_PATH_PREFIX).unwrap_or(path) +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +struct SkillCatalogEntryKey { + authority: SkillAuthority, + package: SkillPackageId, +} + +impl From<&SkillCatalogEntry> for SkillCatalogEntryKey { + fn from(entry: &SkillCatalogEntry) -> Self { + Self { + authority: entry.authority.clone(), + package: entry.id.clone(), + } + } +} diff --git a/codex-rs/ext/skills/src/sources.rs b/codex-rs/ext/skills/src/sources.rs new file mode 100644 index 000000000..9049fcb55 --- /dev/null +++ b/codex-rs/ext/skills/src/sources.rs @@ -0,0 +1,183 @@ +use std::fmt; +use std::sync::Arc; + +use crate::catalog::SkillCatalog; +use crate::catalog::SkillProviderError; +use crate::catalog::SkillReadResult; +use crate::catalog::SkillSearchResult; +use crate::catalog::SkillSourceKind; +use crate::provider::SkillListQuery; +use crate::provider::SkillProvider; +use crate::provider::SkillReadRequest; +use crate::provider::SkillSearchRequest; + +#[derive(Clone)] +pub struct SkillProviderSource { + kind: SkillSourceKind, + label: String, + provider: Arc, +} + +impl SkillProviderSource { + pub fn new( + kind: SkillSourceKind, + label: impl Into, + provider: Arc, + ) -> Self { + Self { + kind, + label: label.into(), + provider, + } + } + + pub fn host(label: impl Into, provider: Arc) -> Self { + Self::new(SkillSourceKind::Host, label, provider) + } + + pub fn executor(label: impl Into, provider: Arc) -> Self { + Self::new(SkillSourceKind::Executor, label, provider) + } + + pub fn remote(label: impl Into, provider: Arc) -> Self { + Self::new(SkillSourceKind::Remote, label, provider) + } + + fn should_list(&self, query: &SkillListQuery) -> bool { + match &self.kind { + SkillSourceKind::Host => query.include_host_skills, + SkillSourceKind::Executor => !query.executor_authorities.is_empty(), + SkillSourceKind::Remote => query.include_remote_skills, + SkillSourceKind::Custom(_) => true, + } + } + + fn owns_kind(&self, kind: &SkillSourceKind) -> bool { + &self.kind == kind + } +} + +impl fmt::Debug for SkillProviderSource { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("SkillProviderSource") + .field("kind", &self.kind) + .field("label", &self.label) + .finish() + } +} + +#[derive(Clone, Default, Debug)] +pub struct SkillProviders { + sources: Vec, +} + +impl SkillProviders { + pub fn new() -> Self { + Self::default() + } + + pub fn with_provider(mut self, source: SkillProviderSource) -> Self { + self.sources.push(source); + self + } + + pub fn with_host_provider(mut self, provider: Arc) -> Self { + self.sources + .push(SkillProviderSource::host("host", provider)); + self + } + + pub fn with_executor_provider(mut self, provider: Arc) -> Self { + self.sources + .push(SkillProviderSource::executor("executor", provider)); + self + } + + pub fn with_remote_provider(mut self, provider: Arc) -> Self { + self.sources + .push(SkillProviderSource::remote("remote", provider)); + self + } + + pub(crate) async fn list_for_turn(&self, query: SkillListQuery) -> SkillCatalog { + let mut catalog = SkillCatalog::default(); + + for source in self + .sources + .iter() + .filter(|source| source.should_list(&query)) + { + extend_catalog( + &mut catalog, + source.provider.list(query.clone()).await, + source.label.as_str(), + ); + } + + catalog + } + + pub(crate) async fn read( + &self, + request: SkillReadRequest, + ) -> Result { + let mut last_error = None; + for source in self + .sources + .iter() + .filter(|source| source.owns_kind(&request.authority.kind)) + { + match source.provider.read(request.clone()).await { + Ok(result) => return Ok(result), + Err(err) => last_error = Some(err), + } + } + + match last_error { + Some(err) => Err(err), + None => Err(SkillProviderError::new(format!( + "{} skill provider is not configured", + request.authority.kind + ))), + } + } + + pub async fn search( + &self, + request: SkillSearchRequest, + ) -> Result { + let mut last_error = None; + for source in self + .sources + .iter() + .filter(|source| source.owns_kind(&request.authority.kind)) + { + match source.provider.search(request.clone()).await { + Ok(result) => return Ok(result), + Err(err) => last_error = Some(err), + } + } + + match last_error { + Some(err) => Err(err), + None => Err(SkillProviderError::new(format!( + "{} skill provider is not configured", + request.authority.kind + ))), + } + } +} + +fn extend_catalog( + catalog: &mut SkillCatalog, + result: Result, + label: &str, +) { + match result { + Ok(source_catalog) => catalog.extend(source_catalog), + Err(err) => catalog + .warnings + .push(format!("{label} skills unavailable: {}", err.message)), + } +} diff --git a/codex-rs/ext/skills/src/state.rs b/codex-rs/ext/skills/src/state.rs index d6c355ed3..4a639eb5f 100644 --- a/codex-rs/ext/skills/src/state.rs +++ b/codex-rs/ext/skills/src/state.rs @@ -1,6 +1,8 @@ use codex_core::config::Config; +use std::sync::Mutex; use crate::catalog::SkillCatalog; +use crate::catalog::SkillCatalogEntry; #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct SkillsExtensionConfig { @@ -17,8 +19,37 @@ impl SkillsExtensionConfig { } } +#[derive(Debug)] +pub(crate) struct SkillsThreadState { + config: Mutex, +} + +impl SkillsThreadState { + pub(crate) fn new(config: SkillsExtensionConfig) -> Self { + Self { + config: Mutex::new(config), + } + } + + pub(crate) fn config(&self) -> SkillsExtensionConfig { + self.config + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + } + + pub(crate) fn set_config(&self, config: SkillsExtensionConfig) { + *self + .config + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = config; + } +} + #[derive(Clone, Debug, Default, PartialEq, Eq)] pub(crate) struct SkillsTurnState { pub(crate) catalog: SkillCatalog, - pub(crate) entrypoints_injected: bool, + pub(crate) selected_entries: Vec, + pub(crate) warnings: Vec, + pub(crate) main_prompts_injected: bool, } diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs new file mode 100644 index 000000000..acdb5587b --- /dev/null +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -0,0 +1,300 @@ +use std::sync::Arc; +use std::sync::Mutex; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; + +use codex_core::config::Config; +use codex_extension_api::ExtensionData; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::ThreadStartInput; +use codex_extension_api::TurnInputContext; +use codex_extension_api::TurnInputEnvironment; +use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; +use codex_protocol::protocol::SessionSource; +use codex_protocol::user_input::UserInput; +use codex_skills_extension::SkillProviders; +use codex_skills_extension::catalog::SkillAuthority; +use codex_skills_extension::catalog::SkillCatalog; +use codex_skills_extension::catalog::SkillCatalogEntry; +use codex_skills_extension::catalog::SkillPackageId; +use codex_skills_extension::catalog::SkillReadResult; +use codex_skills_extension::catalog::SkillResourceId; +use codex_skills_extension::catalog::SkillSearchResult; +use codex_skills_extension::catalog::SkillSourceKind; +use codex_skills_extension::install_with_providers; +use codex_skills_extension::provider::SkillListQuery; +use codex_skills_extension::provider::SkillProvider; +use codex_skills_extension::provider::SkillProviderFuture; +use codex_skills_extension::provider::SkillReadRequest; +use codex_skills_extension::provider::SkillSearchRequest; +use pretty_assertions::assert_eq; + +type TestResult = Result<(), Box>; + +static NEXT_CODEX_HOME_ID: AtomicUsize = AtomicUsize::new(0); + +#[tokio::test] +async fn installed_extension_injects_available_catalog_and_selected_entrypoint() -> TestResult { + let host_read_requests = Arc::new(Mutex::new(Vec::new())); + let remote_read_requests = Arc::new(Mutex::new(Vec::new())); + let host_provider = Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![test_entry( + SkillSourceKind::Host, + "host", + "host/lint-fix", + "lint-fix/SKILL.md", + )], + warnings: Vec::new(), + }, + read_requests: Arc::clone(&host_read_requests), + }); + let remote_provider = Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![test_entry( + SkillSourceKind::Remote, + "remote", + "remote/lint-fix", + "lint-fix/SKILL.md", + )], + warnings: Vec::new(), + }, + read_requests: Arc::clone(&remote_read_requests), + }); + let providers = SkillProviders::new() + .with_host_provider(host_provider) + .with_remote_provider(remote_provider); + let mut builder = ExtensionRegistryBuilder::new(); + install_with_providers(&mut builder, providers); + 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().await?; + registry.thread_lifecycle_contributors()[0] + .on_thread_start(ThreadStartInput { + config: &config, + session_source: &session_source, + persistent_thread_state_available: true, + 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 { + turn_id: "turn-1".to_string(), + user_input: vec![UserInput::Text { + text: "$lint-fix please".to_string(), + text_elements: Vec::new(), + }], + environments: vec![TurnInputEnvironment { + environment_id: "env-1".to_string(), + cwd: std::env::temp_dir(), + is_primary: true, + }], + }, + &session_store, + &thread_store, + &turn_store, + ) + .await; + + assert_eq!(2, fragments.len()); + assert_eq!("developer", fragments[0].role()); + assert!( + fragments[0] + .render() + .starts_with(SKILLS_INSTRUCTIONS_OPEN_TAG) + ); + assert!(fragments[0].render().contains("lint-fix")); + assert_eq!("user", fragments[1].role()); + assert!(fragments[1].render().contains("lint-fix")); + assert!(fragments[1].render().contains("# Lint Fix")); + assert_eq!( + vec![SkillReadRequest { + authority: SkillAuthority::new(SkillSourceKind::Host, "host"), + package: SkillPackageId("host/lint-fix".to_string()), + resource: SkillResourceId("lint-fix/SKILL.md".to_string()), + }], + host_read_requests + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + ); + assert!( + remote_read_requests + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .is_empty() + ); + + let next_turn_store = ExtensionData::new("turn-2"); + let next_fragments = registry.turn_input_contributors()[0] + .contribute( + TurnInputContext { + turn_id: "turn-2".to_string(), + user_input: vec![UserInput::Text { + text: "no skill this time".to_string(), + text_elements: Vec::new(), + }], + environments: Vec::new(), + }, + &session_store, + &thread_store, + &next_turn_store, + ) + .await; + + assert_eq!(1, next_fragments.len()); + assert_eq!("developer", next_fragments[0].role()); + assert!(next_fragments[0].render().contains("lint-fix")); + + Ok(()) +} + +#[tokio::test] +async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { + let read_requests = Arc::new(Mutex::new(Vec::new())); + let provider = Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![ + test_entry( + SkillSourceKind::Host, + "host", + "host/visible-skill", + "visible-skill/SKILL.md", + ), + test_entry( + SkillSourceKind::Host, + "host", + "host/hidden-skill", + "hidden-skill/SKILL.md", + ) + .hidden_from_prompt(), + ], + warnings: Vec::new(), + }, + read_requests: Arc::clone(&read_requests), + }); + let providers = SkillProviders::new().with_host_provider(provider); + let mut builder = ExtensionRegistryBuilder::new(); + install_with_providers(&mut builder, providers); + 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().await?; + registry.thread_lifecycle_contributors()[0] + .on_thread_start(ThreadStartInput { + config: &config, + session_source: &session_source, + persistent_thread_state_available: true, + session_store: &session_store, + thread_store: &thread_store, + }) + .await; + + let fragments = registry.turn_input_contributors()[0] + .contribute( + TurnInputContext { + turn_id: "turn-1".to_string(), + user_input: vec![UserInput::Text { + text: "$hidden-skill".to_string(), + text_elements: Vec::new(), + }], + environments: Vec::new(), + }, + &session_store, + &thread_store, + &ExtensionData::new("turn-1"), + ) + .await; + + assert_eq!(2, fragments.len()); + let catalog_fragment = fragments[0].render(); + assert!(catalog_fragment.contains("visible-skill")); + assert!(!catalog_fragment.contains("hidden-skill")); + assert!(fragments[1].render().contains("hidden-skill")); + assert_eq!( + vec![SkillReadRequest { + authority: SkillAuthority::new(SkillSourceKind::Host, "host"), + package: SkillPackageId("host/hidden-skill".to_string()), + resource: SkillResourceId("hidden-skill/SKILL.md".to_string()), + }], + read_requests + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + ); + + Ok(()) +} + +#[derive(Clone)] +struct StaticSkillProvider { + catalog: SkillCatalog, + read_requests: Arc>>, +} + +impl SkillProvider for StaticSkillProvider { + fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { + let catalog = self.catalog.clone(); + Box::pin(async move { + assert!(query.include_host_skills); + assert!(query.include_bundled_skills); + Ok(catalog) + }) + } + + fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { + let read_requests = Arc::clone(&self.read_requests); + Box::pin(async move { + read_requests + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(request.clone()); + Ok(SkillReadResult { + resource: request.resource, + contents: "# Lint Fix\n\nRun the formatter.".to_string(), + }) + }) + } + + fn search(&self, _request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult> { + Box::pin(async { Ok(SkillSearchResult::default()) }) + } +} + +fn test_entry( + kind: SkillSourceKind, + authority_id: &str, + package_id: &str, + main_prompt: &str, +) -> SkillCatalogEntry { + let name = package_id.rsplit('/').next().unwrap_or(package_id); + SkillCatalogEntry::new( + SkillPackageId(package_id.to_string()), + SkillAuthority::new(kind, authority_id), + name, + "Fix lint errors.", + SkillResourceId(main_prompt.to_string()), + ) + .with_display_path(format!("skill://{package_id}/SKILL.md")) +} + +async fn default_config() -> std::io::Result { + let id = NEXT_CODEX_HOME_ID.fetch_add(1, Ordering::Relaxed); + let codex_home = std::env::temp_dir().join(format!( + "codex-skills-extension-test-{}-{id}", + std::process::id(), + )); + std::fs::create_dir_all(&codex_home)?; + let config = + Config::load_default_with_cli_overrides_for_codex_home(codex_home.clone(), vec![]).await?; + std::fs::remove_dir_all(codex_home)?; + Ok(config) +}