Reintroduce feature flags for skills. (#8244)
1. Reintroduce feature flags for skills; 2. UI tweaks (truncate descriptions, better validation error display).
This commit is contained in:
parent
e1deeefa0f
commit
5c8d22138a
4 changed files with 68 additions and 32 deletions
|
|
@ -219,7 +219,10 @@ impl Codex {
|
|||
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
|
||||
let (tx_event, rx_event) = async_channel::unbounded();
|
||||
|
||||
let loaded_skills = Some(skills_manager.skills_for_cwd(&config.cwd));
|
||||
let loaded_skills = config
|
||||
.features
|
||||
.enabled(Feature::Skills)
|
||||
.then(|| skills_manager.skills_for_cwd(&config.cwd));
|
||||
|
||||
if let Some(outcome) = &loaded_skills {
|
||||
for err in &outcome.errors {
|
||||
|
|
@ -1691,6 +1694,7 @@ mod handlers {
|
|||
|
||||
use crate::codex::spawn_review_thread;
|
||||
use crate::config::Config;
|
||||
use crate::features::Feature;
|
||||
use crate::mcp::auth::compute_auth_statuses;
|
||||
use crate::mcp::collect_mcp_snapshot_from_manager;
|
||||
use crate::review_prompts::resolve_review_request;
|
||||
|
|
@ -1970,20 +1974,29 @@ mod handlers {
|
|||
} else {
|
||||
cwds
|
||||
};
|
||||
let skills_manager = &sess.services.skills_manager;
|
||||
let skills = cwds
|
||||
.into_iter()
|
||||
.map(|cwd| {
|
||||
let outcome = skills_manager.skills_for_cwd_with_options(&cwd, force_reload);
|
||||
let errors = super::errors_to_info(&outcome.errors);
|
||||
let skills = super::skills_to_info(&outcome.skills);
|
||||
SkillsListEntry {
|
||||
let skills = if sess.enabled(Feature::Skills) {
|
||||
let skills_manager = &sess.services.skills_manager;
|
||||
cwds.into_iter()
|
||||
.map(|cwd| {
|
||||
let outcome = skills_manager.skills_for_cwd_with_options(&cwd, force_reload);
|
||||
let errors = super::errors_to_info(&outcome.errors);
|
||||
let skills = super::skills_to_info(&outcome.skills);
|
||||
SkillsListEntry {
|
||||
cwd,
|
||||
skills,
|
||||
errors,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
} else {
|
||||
cwds.into_iter()
|
||||
.map(|cwd| SkillsListEntry {
|
||||
cwd,
|
||||
skills,
|
||||
errors,
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
skills: Vec::new(),
|
||||
errors: Vec::new(),
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::ListSkillsResponse(ListSkillsResponseEvent { skills }),
|
||||
|
|
@ -2228,11 +2241,11 @@ pub(crate) async fn run_task(
|
|||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
|
||||
let skills_outcome = Some(
|
||||
let skills_outcome = sess.enabled(Feature::Skills).then(|| {
|
||||
sess.services
|
||||
.skills_manager
|
||||
.skills_for_cwd(&turn_context.cwd),
|
||||
);
|
||||
.skills_for_cwd(&turn_context.cwd)
|
||||
});
|
||||
|
||||
let SkillInjections {
|
||||
items: skill_items,
|
||||
|
|
|
|||
|
|
@ -83,6 +83,8 @@ pub enum Feature {
|
|||
ShellSnapshot,
|
||||
/// Experimental TUI v2 (viewport) implementation.
|
||||
Tui2,
|
||||
/// Enable discovery and injection of skills.
|
||||
Skills,
|
||||
}
|
||||
|
||||
impl Feature {
|
||||
|
|
@ -381,6 +383,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
|||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Skills,
|
||||
key: "skills",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ShellSnapshot,
|
||||
key: "shell_snapshot",
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ use codex_core::WireApi;
|
|||
use codex_core::auth::AuthCredentialsStoreMode;
|
||||
use codex_core::built_in_model_providers;
|
||||
use codex_core::error::CodexErr;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::openai_models::models_manager::ModelsManager;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
|
|
@ -673,6 +674,7 @@ async fn skills_append_to_instructions() {
|
|||
let mut config = load_default_config_for_test(&codex_home);
|
||||
config.model_provider = model_provider;
|
||||
config.cwd = codex_home.path().to_path_buf();
|
||||
config.features.enable(Feature::Skills);
|
||||
|
||||
let conversation_manager = ConversationManager::with_models_provider_and_home(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
|
|
@ -32,9 +33,13 @@ async fn user_turn_includes_skill_instructions() -> Result<()> {
|
|||
|
||||
let server = start_mock_server().await;
|
||||
let skill_body = "skill body";
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
write_skill(home, "demo", "demo skill", skill_body);
|
||||
});
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Skills);
|
||||
})
|
||||
.with_pre_build_hook(|home| {
|
||||
write_skill(home, "demo", "demo skill", skill_body);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let skill_path = test.codex_home_path().join("skills/demo/SKILL.md");
|
||||
|
|
@ -98,11 +103,15 @@ async fn skill_load_errors_surface_in_session_configured() -> Result<()> {
|
|||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
let skill_dir = home.join("skills").join("broken");
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
fs::write(skill_dir.join("SKILL.md"), "not yaml").unwrap();
|
||||
});
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Skills);
|
||||
})
|
||||
.with_pre_build_hook(|home| {
|
||||
let skill_dir = home.join("skills").join("broken");
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
fs::write(skill_dir.join("SKILL.md"), "not yaml").unwrap();
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
|
|
@ -150,13 +159,17 @@ async fn list_skills_includes_system_cache_entries() -> Result<()> {
|
|||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
let system_skill_path = home.join("skills/.system/plan/SKILL.md");
|
||||
assert!(
|
||||
!system_skill_path.exists(),
|
||||
"expected embedded system skills not yet installed, but {system_skill_path:?} exists"
|
||||
);
|
||||
});
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Skills);
|
||||
})
|
||||
.with_pre_build_hook(|home| {
|
||||
let system_skill_path = home.join("skills/.system/plan/SKILL.md");
|
||||
assert!(
|
||||
!system_skill_path.exists(),
|
||||
"expected embedded system skills not yet installed, but {system_skill_path:?} exists"
|
||||
);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let system_skill_path = test.codex_home_path().join("skills/.system/plan/SKILL.md");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue