Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion codex-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions codex-rs/app-server/src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ where
.with_orchestrator_provider(Arc::new(
codex_skills_extension::OrchestratorSkillProvider::new(),
)),
|config: &Config| codex_skills_extension::SkillsExtensionConfig {
include_instructions: config.include_skill_instructions,
bundled_skills_enabled: config.bundled_skills_enabled(),
},
);
Arc::new(builder.build())
}
Expand Down
1 change: 0 additions & 1 deletion codex-rs/ext/skills/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ doctest = false
workspace = true

[dependencies]
codex-core = { workspace = true }
codex-core-skills = { workspace = true }
codex-exec-server = { workspace = true }
codex-extension-api = { workspace = true }
Expand Down
8 changes: 8 additions & 0 deletions codex-rs/ext/skills/src/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/// Host-supplied configuration used by the skills extension.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct SkillsExtensionConfig {
/// Whether the available-skills catalog is included in model context.
pub include_instructions: bool,
/// Whether bundled skills are eligible for discovery.
pub bundled_skills_enabled: bool,
}
72 changes: 46 additions & 26 deletions codex-rs/ext/skills/src/extension.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use std::sync::Arc;

use codex_core::config::Config;
use codex_core_skills::HostLoadedSkills;
use codex_core_skills::SkillInstructions;
use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_core_skills::injection::SkillInjection;
use codex_extension_api::ConfigContributor;
use codex_extension_api::ContextContributor;
use codex_extension_api::ContextualUserFragment;
Expand All @@ -26,10 +23,12 @@ use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::WarningEvent;

use crate::SkillsExtensionConfig;
use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillReadResult;
use crate::catalog::SkillSourceKind;
use crate::fragments::SkillInstructions;
use crate::provider::HostSkillProvider;
use crate::provider::SkillListQuery;
use crate::provider::SkillReadRequest;
Expand All @@ -40,45 +39,47 @@ use crate::render::truncate_main_prompt_contents;
use crate::render::truncate_utf8_to_bytes;
use crate::selection::collect_explicit_skill_mentions;
use crate::sources::SkillProviders;
use crate::state::SkillsExtensionConfig;
use crate::state::SkillsThreadState;
use crate::state::SkillsTurnState;
use crate::tools::skill_tools;

#[derive(Clone)]
struct SkillsExtension {
struct SkillsExtension<C> {
providers: SkillProviders,
event_sink: Arc<dyn ExtensionEventSink>,
config_from_host: Arc<dyn Fn(&C) -> SkillsExtensionConfig + Send + Sync>,
}

impl ThreadLifecycleContributor<Config> for SkillsExtension {
fn on_thread_start<'a>(
&'a self,
input: ThreadStartInput<'a, Config>,
) -> ExtensionFuture<'a, ()> {
impl<C> ThreadLifecycleContributor<C> for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn on_thread_start<'a>(&'a self, input: ThreadStartInput<'a, C>) -> ExtensionFuture<'a, ()> {
Box::pin(async move {
let selected_roots = input
.thread_store
.get::<Vec<SelectedCapabilityRoot>>()
.map(|selected_roots| selected_roots.as_ref().clone())
.unwrap_or_default();
input.thread_store.insert(SkillsThreadState::new(
SkillsExtensionConfig::from_config(input.config),
(self.config_from_host)(input.config),
selected_roots,
));
})
}
}

impl ConfigContributor<Config> for SkillsExtension {
impl<C> ConfigContributor<C> for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn on_config_changed(
&self,
_session_store: &ExtensionData,
thread_store: &ExtensionData,
_previous_config: &Config,
new_config: &Config,
_previous_config: &C,
new_config: &C,
) {
let next_config = SkillsExtensionConfig::from_config(new_config);
let next_config = (self.config_from_host)(new_config);
if let Some(state) = thread_store.get::<SkillsThreadState>() {
state.set_config(next_config);
} else {
Expand All @@ -87,7 +88,10 @@ impl ConfigContributor<Config> for SkillsExtension {
}
}

impl ContextContributor for SkillsExtension {
impl<C> ContextContributor for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn contribute<'a>(
&'a self,
session_store: &'a ExtensionData,
Expand Down Expand Up @@ -126,7 +130,10 @@ impl ContextContributor for SkillsExtension {
}
}

impl ToolContributor for SkillsExtension {
impl<C> ToolContributor for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn tools(
&self,
session_store: &ExtensionData,
Expand All @@ -143,7 +150,10 @@ impl ToolContributor for SkillsExtension {
}
}

impl TurnInputContributor for SkillsExtension {
impl<C> TurnInputContributor for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn contribute<'a>(
&'a self,
input: TurnInputContext,
Expand Down Expand Up @@ -204,7 +214,7 @@ impl TurnInputContributor for SkillsExtension {
self.emit_warning(&input.turn_id, warning.clone());
warnings.push(warning);
}
let injection = SkillInjection {
let fragment = SkillInstructions {
name: truncate_utf8_to_bytes(&entry.name, MAX_SKILL_NAME_BYTES).0,
path: truncate_utf8_to_bytes(
entry.rendered_path(),
Expand All @@ -213,7 +223,7 @@ impl TurnInputContributor for SkillsExtension {
.0,
contents,
};
fragments.push(Box::new(SkillInstructions::from(&injection)));
fragments.push(Box::new(fragment));
main_prompts_injected = true;
if entry.authority.kind == SkillSourceKind::Host {
injected_host_skill_prompts.insert_path(entry.main_prompt.as_str());
Expand Down Expand Up @@ -259,7 +269,7 @@ impl TurnInputContributor for SkillsExtension {
}
}

impl SkillsExtension {
impl<C> SkillsExtension<C> {
async fn list_skills(
&self,
mut query: SkillListQuery,
Expand Down Expand Up @@ -308,20 +318,30 @@ impl SkillsExtension {
}
}

pub fn install(registry: &mut ExtensionRegistryBuilder<Config>) {
pub fn install<C>(
registry: &mut ExtensionRegistryBuilder<C>,
config_from_host: impl Fn(&C) -> SkillsExtensionConfig + Send + Sync + 'static,
) where
C: Send + Sync + 'static,
{
install_with_providers(
registry,
SkillProviders::new().with_host_provider(Arc::new(HostSkillProvider::new())),
config_from_host,
);
}

pub fn install_with_providers(
registry: &mut ExtensionRegistryBuilder<Config>,
pub fn install_with_providers<C>(
registry: &mut ExtensionRegistryBuilder<C>,
providers: SkillProviders,
) {
config_from_host: impl Fn(&C) -> SkillsExtensionConfig + Send + Sync + 'static,
) where
C: Send + Sync + 'static,
{
let extension = Arc::new(SkillsExtension {
providers,
event_sink: registry.event_sink(),
config_from_host: Arc::new(config_from_host),
});
registry.thread_lifecycle_contributor(extension.clone());
registry.config_contributor(extension.clone());
Expand Down
61 changes: 61 additions & 0 deletions codex-rs/ext/skills/src/fragments.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
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;

#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) struct AvailableSkillsInstructions {
skill_lines: Vec<String>,
}
Comment on lines +7 to +9

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Move injected fragments under core/context

These injected context fragments live in ext/skills instead of core/context (extension.rs:120-121 sends the available-skills fragment; extension.rs:212-217 sends the skill-body fragment). That breaks the context-fragment guidance; move/reuse the structs under core/context.

Useful? React with 👍 / 👎.


impl AvailableSkillsInstructions {
pub(crate) fn from_skill_lines(skill_lines: Vec<String>) -> Self {
Self { skill_lines }
}
}

impl ContextualUserFragment for AvailableSkillsInstructions {
fn role(&self) -> &'static str {
"developer"
}

fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}

fn type_markers() -> (&'static str, &'static str) {
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG)
}

fn body(&self) -> String {
render_available_skills_body(&[], &self.skill_lines)
}
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) struct SkillInstructions {
pub(crate) name: String,
pub(crate) path: String,
pub(crate) contents: String,
}

impl ContextualUserFragment for SkillInstructions {
Comment thread
jif-oai marked this conversation as resolved.
fn role(&self) -> &'static str {
"user"
}

fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}

fn type_markers() -> (&'static str, &'static str) {
("<skill>", "</skill>")
}

fn body(&self) -> String {
let name = &self.name;
let path = &self.path;
let contents = &self.contents;
format!("\n<name>{name}</name>\n<path>{path}</path>\n{contents}\n")
}
}
3 changes: 3 additions & 0 deletions codex-rs/ext/skills/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
pub mod catalog;
mod config;
mod extension;
mod fragments;
pub mod provider;
mod render;
mod selection;
mod sources;
mod state;
mod tools;

pub use config::SkillsExtensionConfig;
pub use extension::install;
pub use extension::install_with_providers;
pub use provider::ExecutorSkillProvider;
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/ext/skills/src/render.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use codex_core::context::AvailableSkillsInstructions;
use codex_utils_string::take_bytes_at_char_boundary;

use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillSourceKind;
use crate::fragments::AvailableSkillsInstructions;

const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000;
const MAX_MAIN_PROMPT_BYTES: usize = 8_000;
Expand Down
17 changes: 1 addition & 16 deletions codex-rs/ext/skills/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,13 @@
use codex_core::config::Config;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use std::future::Future;
use std::sync::Mutex;
use tokio::sync::OnceCell;

use crate::SkillsExtensionConfig;
use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillProviderError;

#[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(Debug)]
pub(crate) struct SkillsThreadState {
config: Mutex<SkillsExtensionConfig>,
Expand Down
Loading
Loading