Files
codex/codex-rs/core/tests/suite/skills.rs
pakrym-oai 136e442e95 [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`
2026-05-05 18:58:18 -07:00

124 lines
4.1 KiB
Rust

#![cfg(not(target_os = "windows"))]
#![allow(clippy::unwrap_used, clippy::expect_used)]
use anyhow::Result;
use codex_exec_server::CreateDirectoryOptions;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Op;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::sse;
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 std::sync::Arc;
async fn write_repo_skill(
cwd: AbsolutePathBuf,
fs: Arc<dyn ExecutorFileSystem>,
name: &str,
description: &str,
body: &str,
) -> Result<()> {
let skill_dir = cwd.join(".agents").join("skills").join(name);
fs.create_directory(
&skill_dir,
CreateDirectoryOptions { recursive: true },
/*sandbox*/ None,
)
.await?;
let contents = format!("---\nname: {name}\ndescription: {description}\n---\n\n{body}\n");
let path = skill_dir.join("SKILL.md");
fs.write_file(&path, contents.into_bytes(), /*sandbox*/ None)
.await?;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn user_turn_includes_skill_instructions() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let skill_body = "skill body";
let mut builder = test_codex().with_workspace_setup(move |cwd, fs| async move {
write_repo_skill(cwd, fs, "demo", "demo skill", skill_body).await
});
let test = builder.build_remote_aware(&server).await?;
let skill_path = test
.config
.cwd
.join(".agents/skills/demo/SKILL.md")
.canonicalize()
.unwrap_or_else(|_| test.config.cwd.join(".agents/skills/demo/SKILL.md"))
.to_path_buf();
let mock = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-1"),
]),
)
.await;
let session_model = test.session_configured.model.clone();
let (sandbox_policy, permission_profile) =
turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path());
test.codex
.submit(Op::UserTurn {
environments: None,
items: vec![
UserInput::Text {
text: "please use $demo".to_string(),
text_elements: Vec::new(),
},
UserInput::Skill {
name: "demo".to_string(),
path: skill_path.clone(),
},
],
final_output_json_schema: None,
cwd: test.config.cwd.to_path_buf(),
approval_policy: AskForApproval::Never,
approvals_reviewer: None,
sandbox_policy,
permission_profile,
model: session_model,
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
})
.await?;
core_test_support::wait_for_event(test.codex.as_ref(), |event| {
matches!(event, codex_protocol::protocol::EventMsg::TurnComplete(_))
})
.await;
let request = mock.single_request();
let user_texts = request.message_input_texts("user");
let skill_path_str = skill_path.to_string_lossy();
assert!(
user_texts.iter().any(|text| {
text.contains("<skill>\n<name>demo</name>")
&& text.contains("<path>")
&& text.contains(skill_body)
&& text.contains(skill_path_str.as_ref())
}),
"expected skill instructions in user input, got {user_texts:?}"
);
Ok(())
}