mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Remove legacy ListSkills op (#21282)
## Why `skills/list` is already exposed through app-server v2 and covered by the app-server test suite. Keeping the separate core `Op::ListSkills` path leaves a duplicate legacy protocol surface that no longer needs to be maintained. ## What Changed - Removed `Op::ListSkills` and `EventMsg::ListSkillsResponse` from the core protocol. - Deleted the corresponding core session handler and stale core integration tests. - Removed rollout/MCP ignore branches and protocol v1 docs references for the deleted event/op. - Left app-server `skills/list` and its existing coverage intact. ## Validation - `cargo test -p codex-protocol` - `cargo test -p codex-core --test all suite::skills` - `cargo check -p codex-mcp-server -p codex-rollout -p codex-rollout-trace` - `just fix -p codex-core`
This commit is contained in:
committed by
GitHub
Unverified
parent
024118625e
commit
136e442e95
@@ -89,11 +89,9 @@ mod session_prefix;
|
||||
mod session_startup_prewarm;
|
||||
mod shell_detect;
|
||||
pub mod skills;
|
||||
pub(crate) use skills::SkillError;
|
||||
pub(crate) use skills::SkillInjections;
|
||||
pub(crate) use skills::SkillLoadOutcome;
|
||||
pub(crate) use skills::SkillMetadata;
|
||||
pub(crate) use skills::SkillsLoadInput;
|
||||
pub(crate) use skills::SkillsManager;
|
||||
pub(crate) use skills::build_available_skills;
|
||||
pub(crate) use skills::build_skill_injections;
|
||||
|
||||
@@ -18,14 +18,8 @@ use crate::realtime_context::REALTIME_TURN_TOKEN_BUDGET;
|
||||
use crate::realtime_context::truncate_realtime_text_to_token_budget;
|
||||
use crate::realtime_conversation::REALTIME_USER_TEXT_PREFIX;
|
||||
use crate::realtime_conversation::prefix_realtime_v2_text;
|
||||
use crate::session::spawn_review_thread;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::loader::load_config_layers_state;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
use crate::review_prompts::resolve_review_request;
|
||||
use crate::session::spawn_review_thread;
|
||||
use crate::tasks::CompactTask;
|
||||
use crate::tasks::UserShellCommandMode;
|
||||
use crate::tasks::UserShellCommandTask;
|
||||
@@ -41,7 +35,6 @@ use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::GuardianAssessmentEvent;
|
||||
use codex_protocol::protocol::GuardianAssessmentStatus;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use codex_protocol::protocol::ListSkillsResponseEvent;
|
||||
use codex_protocol::protocol::McpServerRefreshConfig;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::RealtimeConversationListVoicesResponseEvent;
|
||||
@@ -49,8 +42,6 @@ use codex_protocol::protocol::RealtimeVoicesList;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::ReviewRequest;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SkillErrorInfo;
|
||||
use codex_protocol::protocol::SkillsListEntry;
|
||||
use codex_protocol::protocol::ThreadMemoryMode;
|
||||
use codex_protocol::protocol::ThreadRolledBackEvent;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
@@ -69,7 +60,6 @@ use codex_protocol::user_input::UserInput;
|
||||
use codex_rmcp_client::ElicitationAction;
|
||||
use codex_rmcp_client::ElicitationResponse;
|
||||
use serde_json::Value;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tracing::debug;
|
||||
use tracing::info;
|
||||
@@ -555,98 +545,6 @@ pub async fn list_mcp_tools(sess: &Session, config: &Arc<Config>, sub_id: String
|
||||
sess.send_event_raw(event).await;
|
||||
}
|
||||
|
||||
pub async fn list_skills(sess: &Session, sub_id: String, cwds: Vec<PathBuf>, force_reload: bool) {
|
||||
let default_cwd = {
|
||||
let state = sess.state.lock().await;
|
||||
state.session_configuration.cwd.to_path_buf()
|
||||
};
|
||||
let cwds = if cwds.is_empty() {
|
||||
vec![default_cwd]
|
||||
} else {
|
||||
cwds
|
||||
};
|
||||
|
||||
let skills_manager = &sess.services.skills_manager;
|
||||
let plugins_manager = &sess.services.plugins_manager;
|
||||
let fs = sess
|
||||
.services
|
||||
.environment_manager
|
||||
.default_environment()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
let config = sess.get_config().await;
|
||||
let codex_home = sess.codex_home().await;
|
||||
let mut skills = Vec::new();
|
||||
let empty_cli_overrides: &[(String, toml::Value)] = &[];
|
||||
for cwd in cwds {
|
||||
let cwd_abs = match AbsolutePathBuf::relative_to_current_dir(cwd.as_path()) {
|
||||
Ok(path) => path,
|
||||
Err(err) => {
|
||||
let error_path = cwd.clone();
|
||||
skills.push(SkillsListEntry {
|
||||
cwd,
|
||||
skills: Vec::new(),
|
||||
errors: vec![SkillErrorInfo {
|
||||
path: error_path,
|
||||
message: err.to_string(),
|
||||
}],
|
||||
});
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let config_layer_stack = match load_config_layers_state(
|
||||
LOCAL_FS.as_ref(),
|
||||
&codex_home,
|
||||
Some(cwd_abs.clone()),
|
||||
empty_cli_overrides,
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(config_layer_stack) => config_layer_stack,
|
||||
Err(err) => {
|
||||
let error_path = cwd.clone();
|
||||
skills.push(SkillsListEntry {
|
||||
cwd,
|
||||
skills: Vec::new(),
|
||||
errors: vec![SkillErrorInfo {
|
||||
path: error_path,
|
||||
message: err.to_string(),
|
||||
}],
|
||||
});
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let plugins_input = config.plugins_config_input();
|
||||
let effective_skill_roots = plugins_manager
|
||||
.effective_skill_roots_for_layer_stack(&config_layer_stack, &plugins_input)
|
||||
.await;
|
||||
let skills_input = crate::SkillsLoadInput::new(
|
||||
cwd_abs.clone(),
|
||||
effective_skill_roots,
|
||||
config_layer_stack,
|
||||
config.bundled_skills_enabled(),
|
||||
);
|
||||
let outcome = skills_manager
|
||||
.skills_for_cwd(&skills_input, force_reload, fs.clone())
|
||||
.await;
|
||||
let errors = super::errors_to_info(&outcome.errors);
|
||||
let skills_metadata = super::skills_to_info(&outcome.skills, &outcome.disabled_paths);
|
||||
skills.push(SkillsListEntry {
|
||||
cwd,
|
||||
skills: skills_metadata,
|
||||
errors,
|
||||
});
|
||||
}
|
||||
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::ListSkillsResponse(ListSkillsResponseEvent { skills }),
|
||||
};
|
||||
sess.send_event_raw(event).await;
|
||||
}
|
||||
|
||||
pub async fn compact(sess: &Arc<Session>, sub_id: String) {
|
||||
let turn_context = sess.new_default_turn_with_sub_id(sub_id).await;
|
||||
|
||||
@@ -1032,10 +930,6 @@ pub(super) async fn submission_loop(
|
||||
reload_user_config(&sess).await;
|
||||
false
|
||||
}
|
||||
Op::ListSkills { cwds, force_reload } => {
|
||||
list_skills(&sess, sub.id.clone(), cwds, force_reload).await;
|
||||
false
|
||||
}
|
||||
Op::Compact => {
|
||||
compact(&sess, sub.id.clone()).await;
|
||||
false
|
||||
|
||||
@@ -266,8 +266,9 @@ pub(crate) struct PreviousTurnSettings {
|
||||
pub(crate) realtime_active: Option<bool>,
|
||||
}
|
||||
|
||||
use crate::SkillError;
|
||||
#[cfg(test)]
|
||||
use crate::SkillLoadOutcome;
|
||||
#[cfg(test)]
|
||||
use crate::SkillMetadata;
|
||||
use crate::SkillsManager;
|
||||
use crate::agents_md::AgentsMdManager;
|
||||
@@ -342,11 +343,6 @@ use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionConfiguredEvent;
|
||||
use codex_protocol::protocol::SessionNetworkProxyRuntime;
|
||||
use codex_protocol::protocol::SkillDependencies as ProtocolSkillDependencies;
|
||||
use codex_protocol::protocol::SkillErrorInfo;
|
||||
use codex_protocol::protocol::SkillInterface as ProtocolSkillInterface;
|
||||
use codex_protocol::protocol::SkillMetadata as ProtocolSkillMetadata;
|
||||
use codex_protocol::protocol::SkillToolDependency as ProtocolSkillToolDependency;
|
||||
use codex_protocol::protocol::StreamErrorEvent;
|
||||
use codex_protocol::protocol::Submission;
|
||||
use codex_protocol::protocol::ThreadMemoryMode;
|
||||
@@ -984,6 +980,7 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) async fn codex_home(&self) -> AbsolutePathBuf {
|
||||
let state = self.state.lock().await;
|
||||
state.session_configuration.codex_home().clone()
|
||||
@@ -3293,60 +3290,6 @@ pub(crate) fn emit_subagent_session_started(
|
||||
});
|
||||
}
|
||||
|
||||
fn skills_to_info(
|
||||
skills: &[SkillMetadata],
|
||||
disabled_paths: &HashSet<AbsolutePathBuf>,
|
||||
) -> Vec<ProtocolSkillMetadata> {
|
||||
skills
|
||||
.iter()
|
||||
.map(|skill| ProtocolSkillMetadata {
|
||||
name: skill.name.clone(),
|
||||
description: skill.description.clone(),
|
||||
short_description: skill.short_description.clone(),
|
||||
interface: skill
|
||||
.interface
|
||||
.clone()
|
||||
.map(|interface| ProtocolSkillInterface {
|
||||
display_name: interface.display_name,
|
||||
short_description: interface.short_description,
|
||||
icon_small: interface.icon_small,
|
||||
icon_large: interface.icon_large,
|
||||
brand_color: interface.brand_color,
|
||||
default_prompt: interface.default_prompt,
|
||||
}),
|
||||
dependencies: skill.dependencies.clone().map(|dependencies| {
|
||||
ProtocolSkillDependencies {
|
||||
tools: dependencies
|
||||
.tools
|
||||
.into_iter()
|
||||
.map(|tool| ProtocolSkillToolDependency {
|
||||
r#type: tool.r#type,
|
||||
value: tool.value,
|
||||
description: tool.description,
|
||||
transport: tool.transport,
|
||||
command: tool.command,
|
||||
url: tool.url,
|
||||
})
|
||||
.collect(),
|
||||
}
|
||||
}),
|
||||
path: skill.path_to_skills_md.clone(),
|
||||
scope: skill.scope,
|
||||
enabled: !disabled_paths.contains(&skill.path_to_skills_md),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn errors_to_info(errors: &[SkillError]) -> Vec<SkillErrorInfo> {
|
||||
errors
|
||||
.iter()
|
||||
.map(|err| SkillErrorInfo {
|
||||
path: err.path.to_path_buf(),
|
||||
message: err.message.clone(),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
use codex_memories_read::build_memory_tool_developer_instructions;
|
||||
|
||||
/// Builds the hook engine for one config snapshot, including any enabled plugin hooks.
|
||||
|
||||
@@ -1506,7 +1506,6 @@ pub(super) fn realtime_text_for_event(msg: &EventMsg) -> Option<String> {
|
||||
| EventMsg::TurnDiff(_)
|
||||
| EventMsg::GetHistoryEntryResponse(_)
|
||||
| EventMsg::McpListToolsResponse(_)
|
||||
| EventMsg::ListSkillsResponse(_)
|
||||
| EventMsg::RealtimeConversationListVoicesResponse(_)
|
||||
| EventMsg::SkillsUpdateAvailable
|
||||
| EventMsg::PlanUpdate(_)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::SkillLoadOutcome;
|
||||
use crate::config::GhostSnapshotConfig;
|
||||
use crate::environment_selection::ResolvedTurnEnvironments;
|
||||
use codex_model_provider::SharedModelProvider;
|
||||
|
||||
@@ -2,22 +2,13 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::agent_graph_store_from_state_db;
|
||||
use codex_core::init_state_db_from_config;
|
||||
use codex_core::thread_store_from_config;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
@@ -27,11 +18,7 @@ use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_codex::turn_permission_fields;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
|
||||
async fn write_repo_skill(
|
||||
cwd: AbsolutePathBuf,
|
||||
@@ -54,22 +41,6 @@ async fn write_repo_skill(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_home_skill(codex_home: &Path, dir: &str, name: &str, description: &str) -> Result<()> {
|
||||
let skill_dir = codex_home.join("skills").join(dir);
|
||||
fs::create_dir_all(&skill_dir)?;
|
||||
let contents = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
|
||||
fs::write(skill_dir.join("SKILL.md"), contents)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn system_skill_md_path(home: impl AsRef<Path>, name: &str) -> std::path::PathBuf {
|
||||
home.as_ref()
|
||||
.join("skills")
|
||||
.join(".system")
|
||||
.join(name)
|
||||
.join("SKILL.md")
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn user_turn_includes_skill_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -150,267 +121,3 @@ async fn user_turn_includes_skill_instructions() -> Result<()> {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn list_skills_includes_repo_and_home_skills_remote_aware() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(|home| {
|
||||
write_home_skill(home, "home-demo", "home-demo", "from home")
|
||||
.expect("write home skill");
|
||||
})
|
||||
.with_workspace_setup(|cwd, fs| async move {
|
||||
write_repo_skill(cwd, fs, "repo-demo", "from repo", "# Body").await
|
||||
});
|
||||
let test = builder.build_remote_aware(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::ListSkills {
|
||||
cwds: Vec::new(),
|
||||
force_reload: true,
|
||||
})
|
||||
.await?;
|
||||
let response =
|
||||
core_test_support::wait_for_event_match(test.codex.as_ref(), |event| match event {
|
||||
codex_protocol::protocol::EventMsg::ListSkillsResponse(response) => {
|
||||
Some(response.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let cwd = test.config.cwd.as_path();
|
||||
let skills = response
|
||||
.skills
|
||||
.iter()
|
||||
.find(|entry| entry.cwd.as_path() == cwd)
|
||||
.map(|entry| entry.skills.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
let repo_skill = skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == "repo-demo")
|
||||
.expect("expected repo skill");
|
||||
assert_eq!(repo_skill.scope, codex_protocol::protocol::SkillScope::Repo);
|
||||
let repo_path = repo_skill.path.to_string_lossy().replace('\\', "/");
|
||||
assert!(
|
||||
repo_path.ends_with("/.agents/skills/repo-demo/SKILL.md"),
|
||||
"unexpected repo skill path: {repo_path}"
|
||||
);
|
||||
|
||||
let home_skill = skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == "home-demo")
|
||||
.expect("expected home skill");
|
||||
assert_eq!(home_skill.scope, codex_protocol::protocol::SkillScope::User);
|
||||
let home_path = home_skill.path.to_string_lossy().replace('\\', "/");
|
||||
assert!(
|
||||
home_path.ends_with("/skills/home-demo/SKILL.md"),
|
||||
"unexpected home skill path: {home_path}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn list_skills_skips_cwd_roots_when_environment_disabled() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
write_home_skill(
|
||||
codex_home.path(),
|
||||
"home-disabled",
|
||||
"home-disabled",
|
||||
"from home",
|
||||
)?;
|
||||
let repo_skill_dir = cwd
|
||||
.path()
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("repo-disabled");
|
||||
fs::create_dir_all(&repo_skill_dir)?;
|
||||
fs::write(
|
||||
repo_skill_dir.join("SKILL.md"),
|
||||
"---\nname: repo-disabled\ndescription: from repo\n---\n\n# Body\n",
|
||||
)?;
|
||||
let mut config = load_default_config_for_test(&codex_home).await;
|
||||
config.cwd = AbsolutePathBuf::from_absolute_path_checked(cwd.path())?;
|
||||
|
||||
let state_db = init_state_db_from_config(&config)
|
||||
.await
|
||||
.expect("skills test requires state db");
|
||||
let thread_store = thread_store_from_config(&config, state_db.clone());
|
||||
let agent_graph_store = agent_graph_store_from_state_db(state_db.clone());
|
||||
let thread_manager = ThreadManager::new(
|
||||
&config,
|
||||
codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("dummy")),
|
||||
SessionSource::Exec,
|
||||
Arc::new(EnvironmentManager::disabled_for_tests(
|
||||
ExecServerRuntimePaths::new(
|
||||
std::env::current_exe()?,
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
)?,
|
||||
)),
|
||||
/*analytics_events_client*/ None,
|
||||
state_db,
|
||||
thread_store,
|
||||
agent_graph_store,
|
||||
);
|
||||
let new_thread = thread_manager.start_thread(config.clone()).await?;
|
||||
let cwd = config.cwd.to_path_buf();
|
||||
|
||||
new_thread
|
||||
.thread
|
||||
.submit(Op::ListSkills {
|
||||
cwds: vec![cwd.clone()],
|
||||
force_reload: true,
|
||||
})
|
||||
.await?;
|
||||
let response =
|
||||
core_test_support::wait_for_event_match(new_thread.thread.as_ref(), |event| match event {
|
||||
codex_protocol::protocol::EventMsg::ListSkillsResponse(response) => {
|
||||
Some(response.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(response.skills.len(), 1);
|
||||
assert_eq!(response.skills[0].cwd, cwd);
|
||||
assert_eq!(response.skills[0].errors.len(), 0);
|
||||
assert!(
|
||||
response.skills[0]
|
||||
.skills
|
||||
.iter()
|
||||
.any(|skill| skill.name == "home-disabled")
|
||||
);
|
||||
assert!(
|
||||
response.skills[0]
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "repo-disabled")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
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 test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::ListSkills {
|
||||
cwds: Vec::new(),
|
||||
force_reload: false,
|
||||
})
|
||||
.await?;
|
||||
let response =
|
||||
core_test_support::wait_for_event_match(test.codex.as_ref(), |event| match event {
|
||||
codex_protocol::protocol::EventMsg::ListSkillsResponse(response) => {
|
||||
Some(response.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let cwd = test.cwd_path();
|
||||
let (skills, errors) = response
|
||||
.skills
|
||||
.iter()
|
||||
.find(|entry| entry.cwd.as_path() == cwd)
|
||||
.map(|entry| (entry.skills.clone(), entry.errors.clone()))
|
||||
.unwrap_or_default();
|
||||
|
||||
assert!(
|
||||
skills.iter().all(|skill| {
|
||||
!skill
|
||||
.path
|
||||
.to_string_lossy()
|
||||
.ends_with("skills/broken/SKILL.md")
|
||||
}),
|
||||
"expected broken skill not loaded, got {skills:?}"
|
||||
);
|
||||
assert_eq!(errors.len(), 1, "expected one load error");
|
||||
let error_path = errors[0].path.to_string_lossy();
|
||||
assert!(
|
||||
error_path.ends_with("skills/broken/SKILL.md"),
|
||||
"unexpected error path: {error_path}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn list_skills_includes_system_cache_entries() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
const SYSTEM_SKILL_NAME: &str = "skill-creator";
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
let system_skill_path = system_skill_md_path(home, SYSTEM_SKILL_NAME);
|
||||
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 = system_skill_md_path(test.codex_home_path(), SYSTEM_SKILL_NAME);
|
||||
assert!(
|
||||
system_skill_path.exists(),
|
||||
"expected embedded system skills installed to {system_skill_path:?}"
|
||||
);
|
||||
let system_skill_contents = fs::read_to_string(&system_skill_path)?;
|
||||
let expected_name_line = format!("name: {SYSTEM_SKILL_NAME}");
|
||||
assert!(
|
||||
system_skill_contents.contains(&expected_name_line),
|
||||
"expected embedded system skill file, got:\n{system_skill_contents}"
|
||||
);
|
||||
|
||||
test.codex
|
||||
.submit(Op::ListSkills {
|
||||
cwds: Vec::new(),
|
||||
force_reload: true,
|
||||
})
|
||||
.await?;
|
||||
let response =
|
||||
core_test_support::wait_for_event_match(test.codex.as_ref(), |event| match event {
|
||||
codex_protocol::protocol::EventMsg::ListSkillsResponse(response) => {
|
||||
Some(response.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let cwd = test.cwd_path();
|
||||
let (skills, _errors) = response
|
||||
.skills
|
||||
.iter()
|
||||
.find(|entry| entry.cwd.as_path() == cwd)
|
||||
.map(|entry| (entry.skills.clone(), entry.errors.clone()))
|
||||
.unwrap_or_default();
|
||||
|
||||
let skill = skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == SYSTEM_SKILL_NAME)
|
||||
.expect("expected system skill to be present");
|
||||
assert_eq!(skill.scope, codex_protocol::protocol::SkillScope::System);
|
||||
let path_str = skill.path.to_string_lossy().replace('\\', "/");
|
||||
let expected_path_suffix = format!("/skills/.system/{SYSTEM_SKILL_NAME}/SKILL.md");
|
||||
assert!(
|
||||
path_str.ends_with(&expected_path_suffix),
|
||||
"unexpected skill path: {path_str}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -70,7 +70,6 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc
|
||||
- `Op::Interrupt` – Interrupts a running turn
|
||||
- `Op::ExecApproval` – Approve or deny code execution
|
||||
- `Op::UserInputAnswer` – Provide answers for a `request_user_input` tool call
|
||||
- `Op::ListSkills` – Request skills for one or more cwd values (optionally `force_reload`)
|
||||
- `Op::UserTurn` and `Op::OverrideTurnContext` accept an optional `personality` override that updates the model’s communication style
|
||||
|
||||
Valid `personality` values are `friendly`, `pragmatic`, and `none`. When `none` is selected, the personality placeholder is replaced with an empty string.
|
||||
@@ -86,7 +85,6 @@ Valid `personality` values are `friendly`, `pragmatic`, and `none`. When `none`
|
||||
- `EventMsg::Error` – A turn stopped with an error
|
||||
- `EventMsg::Warning` – A non-fatal warning that the client should surface to the user
|
||||
- `EventMsg::TurnComplete` – Contains a `response_id` bookmark for last `response_id` executed by the turn. This can be used to continue the turn at a later point in time, perhaps with additional user input.
|
||||
- `EventMsg::ListSkillsResponse` – Response payload with per-cwd skill entries (`cwd`, `skills`, `errors`)
|
||||
|
||||
### UserInput items
|
||||
|
||||
|
||||
@@ -335,7 +335,6 @@ async fn run_codex_tool_session_inner(
|
||||
| EventMsg::McpToolCallBegin(_)
|
||||
| EventMsg::McpToolCallEnd(_)
|
||||
| EventMsg::McpListToolsResponse(_)
|
||||
| EventMsg::ListSkillsResponse(_)
|
||||
| EventMsg::RealtimeConversationListVoicesResponse(_)
|
||||
| EventMsg::ExecCommandBegin(_)
|
||||
| EventMsg::TerminalInteraction(_)
|
||||
|
||||
@@ -748,19 +748,6 @@ pub enum Op {
|
||||
/// enable/disable state) without restarting the thread.
|
||||
ReloadUserConfig,
|
||||
|
||||
/// Request the list of skills for the provided `cwd` values or the session default.
|
||||
ListSkills {
|
||||
/// Working directories to scope repo skills discovery.
|
||||
///
|
||||
/// When empty, the session default working directory is used.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
cwds: Vec<PathBuf>,
|
||||
|
||||
/// When true, recompute skills even if a cached result exists.
|
||||
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
||||
force_reload: bool,
|
||||
},
|
||||
|
||||
/// Request the agent to summarize the current conversation context.
|
||||
/// The agent will use its existing context (either conversation history or previous response id)
|
||||
/// to generate a summary which will be returned as an AgentMessage event.
|
||||
@@ -893,7 +880,6 @@ impl Op {
|
||||
Self::ListMcpTools => "list_mcp_tools",
|
||||
Self::RefreshMcpServers { .. } => "refresh_mcp_servers",
|
||||
Self::ReloadUserConfig => "reload_user_config",
|
||||
Self::ListSkills { .. } => "list_skills",
|
||||
Self::Compact => "compact",
|
||||
Self::SetThreadMemoryMode { .. } => "set_thread_memory_mode",
|
||||
Self::ThreadRollback { .. } => "thread_rollback",
|
||||
@@ -1441,9 +1427,6 @@ pub enum EventMsg {
|
||||
/// List of MCP tools available to the agent.
|
||||
McpListToolsResponse(McpListToolsResponseEvent),
|
||||
|
||||
/// List of skills available to the agent.
|
||||
ListSkillsResponse(ListSkillsResponseEvent),
|
||||
|
||||
/// List of voices supported by realtime conversation streams.
|
||||
RealtimeConversationListVoicesResponse(RealtimeConversationListVoicesResponseEvent),
|
||||
|
||||
@@ -3312,12 +3295,6 @@ impl fmt::Display for McpAuthStatus {
|
||||
}
|
||||
}
|
||||
|
||||
/// Response payload for `Op::ListSkills`.
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct ListSkillsResponseEvent {
|
||||
pub skills: Vec<SkillsListEntry>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
|
||||
pub struct RealtimeConversationListVoicesResponseEvent {
|
||||
pub voices: RealtimeVoicesList,
|
||||
@@ -3427,19 +3404,6 @@ pub struct SkillToolDependency {
|
||||
pub url: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct SkillErrorInfo {
|
||||
pub path: PathBuf,
|
||||
pub message: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct SkillsListEntry {
|
||||
pub cwd: PathBuf,
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
pub errors: Vec<SkillErrorInfo>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)]
|
||||
pub struct SessionNetworkProxyRuntime {
|
||||
pub http_addr: String,
|
||||
|
||||
@@ -261,7 +261,6 @@ pub(crate) fn tool_runtime_trace_event(event: &EventMsg) -> Option<ToolRuntimeTr
|
||||
| EventMsg::TurnDiff(_)
|
||||
| EventMsg::GetHistoryEntryResponse(_)
|
||||
| EventMsg::McpListToolsResponse(_)
|
||||
| EventMsg::ListSkillsResponse(_)
|
||||
| EventMsg::RealtimeConversationListVoicesResponse(_)
|
||||
| EventMsg::SkillsUpdateAvailable
|
||||
| EventMsg::PlanUpdate(_)
|
||||
@@ -337,7 +336,6 @@ pub(crate) fn wrapped_protocol_event_type(event: &EventMsg) -> Option<&'static s
|
||||
| EventMsg::TurnDiff(_)
|
||||
| EventMsg::GetHistoryEntryResponse(_)
|
||||
| EventMsg::McpListToolsResponse(_)
|
||||
| EventMsg::ListSkillsResponse(_)
|
||||
| EventMsg::RealtimeConversationListVoicesResponse(_)
|
||||
| EventMsg::SkillsUpdateAvailable
|
||||
| EventMsg::PlanUpdate(_)
|
||||
|
||||
@@ -159,7 +159,6 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option<EventPersistenceMode> {
|
||||
| EventMsg::RealtimeConversationListVoicesResponse(_)
|
||||
| EventMsg::McpStartupUpdate(_)
|
||||
| EventMsg::McpStartupComplete(_)
|
||||
| EventMsg::ListSkillsResponse(_)
|
||||
| EventMsg::WebSearchBegin(_)
|
||||
| EventMsg::PlanUpdate(_)
|
||||
| EventMsg::ShutdownComplete
|
||||
|
||||
Reference in New Issue
Block a user