Remove Codex skill config residue

This commit is contained in:
Meta Vens 2026-05-14 03:20:50 +01:00
commit ced7aa44d1
30 changed files with 24 additions and 1622 deletions

View file

@ -910,6 +910,16 @@ for old history classification. Live structured user-input mentions are
MCP-shaped only; MCP remains JSON-RPC at its protocol edge, not a reason to keep
Codex skills alive inside the organ.
The remaining live skill config/protocol/analytics residues are now cut too.
`[skills]` TOML parsing, config schema entries, config edit helpers,
`SkillScope` / `SkillMetadata` protocol structs, skill invocation analytics,
thread skill metric constants, the skill MCP dependency installer, and the stale
skill approval integration suite were deleted. `config.schema.json` was
regenerated without a `skills` table. The remaining skill-shaped stumps are the
separate granular `skill_approval` permission flag and old `<skill>` /
`<skills_instructions>` history/snapshot compatibility surfaces; classify and
cut those next instead of treating them as auth/model spine machinery.
That husk has now been cut from `codex-core`. The root core crate no longer
exports `plugins`, no longer depends on `codex-core-plugins` or `codex-plugin`,
and the core plugin manager / marketplace add-remove-sync modules and tests

Binary file not shown.

File diff suppressed because one or more lines are too long

View file

@ -28,9 +28,6 @@ use crate::facts::CustomAnalyticsFact;
use crate::facts::HookRunFact;
use crate::facts::HookRunInput;
use crate::facts::InputError;
use crate::facts::InvocationType;
use crate::facts::SkillInvocation;
use crate::facts::SkillInvokedInput;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::ThreadInitializationMode;
use crate::facts::TrackEventsContext;
@ -39,8 +36,6 @@ use crate::facts::TurnStatus;
use crate::facts::TurnSteerRequestError;
use crate::facts::TurnTokenUsageFact;
use crate::reducer::AnalyticsReducer;
use crate::reducer::normalize_path_for_skill_id;
use crate::reducer::skill_id_for_local_skill;
use codex_app_server_protocol::ApprovalsReviewer as AppServerApprovalsReviewer;
use codex_app_server_protocol::AskForApproval as AppServerAskForApproval;
use codex_app_server_protocol::ClientInfo;
@ -70,7 +65,6 @@ use codex_app_server_protocol::TurnSteerParams;
use codex_app_server_protocol::TurnSteerResponse;
use codex_app_server_protocol::UserInput;
use codex_login::default_client::DEFAULT_ORIGINATOR;
use codex_login::default_client::originator;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::ModeKind;
@ -87,7 +81,6 @@ use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::path::PathBuf;
fn sample_thread(thread_id: &str, ephemeral: bool) -> Thread {
sample_thread_with_source(thread_id, ephemeral, AppServerSessionSource::Exec)
@ -532,70 +525,6 @@ async fn ingest_turn_prerequisites(
}
}
fn expected_absolute_path(path: &PathBuf) -> String {
std::fs::canonicalize(path)
.unwrap_or_else(|_| path.to_path_buf())
.to_string_lossy()
.replace('\\', "/")
}
#[test]
fn normalize_path_for_skill_id_repo_scoped_uses_relative_path() {
let repo_root = PathBuf::from("/repo/root");
let skill_path = PathBuf::from("/repo/root/.codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(
Some("https://example.com/repo.git"),
Some(repo_root.as_path()),
skill_path.as_path(),
);
assert_eq!(path, ".codex/skills/doc/SKILL.md");
}
#[test]
fn normalize_path_for_skill_id_user_scoped_uses_absolute_path() {
let skill_path = PathBuf::from("/Users/abc/.codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(
/*repo_url*/ None,
/*repo_root*/ None,
skill_path.as_path(),
);
let expected = expected_absolute_path(&skill_path);
assert_eq!(path, expected);
}
#[test]
fn normalize_path_for_skill_id_admin_scoped_uses_absolute_path() {
let skill_path = PathBuf::from("/etc/codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(
/*repo_url*/ None,
/*repo_root*/ None,
skill_path.as_path(),
);
let expected = expected_absolute_path(&skill_path);
assert_eq!(path, expected);
}
#[test]
fn normalize_path_for_skill_id_repo_root_not_in_skill_path_uses_absolute_path() {
let repo_root = PathBuf::from("/repo/root");
let skill_path = PathBuf::from("/other/path/.codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(
Some("https://example.com/repo.git"),
Some(repo_root.as_path()),
skill_path.as_path(),
);
let expected = expected_absolute_path(&skill_path);
assert_eq!(path, expected);
}
#[test]
fn compaction_event_serializes_expected_shape() {
let event = TrackEventRequest::Compaction(Box::new(CodexCompactionEventRequest {
@ -1353,57 +1282,6 @@ fn hook_run_metadata_maps_stopped_status() {
assert_eq!(stopped["status"], "stopped");
}
#[tokio::test]
async fn reducer_ingests_skill_invoked_fact() {
let mut reducer = AnalyticsReducer::default();
let mut events = Vec::new();
let tracking = TrackEventsContext {
model_slug: "gpt-5".to_string(),
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
};
let skill_path = PathBuf::from("/Users/abc/.codex/skills/doc/SKILL.md");
let expected_skill_id = skill_id_for_local_skill(
/*repo_url*/ None,
/*repo_root*/ None,
skill_path.as_path(),
"doc",
);
reducer
.ingest(
AnalyticsFact::Custom(CustomAnalyticsFact::SkillInvoked(SkillInvokedInput {
tracking,
invocations: vec![SkillInvocation {
skill_name: "doc".to_string(),
skill_scope: codex_protocol::protocol::SkillScope::User,
skill_path,
invocation_type: InvocationType::Explicit,
}],
})),
&mut events,
)
.await;
let payload = serde_json::to_value(&events).expect("serialize events");
assert_eq!(
payload,
json!([{
"event_type": "skill_invocation",
"skill_id": expected_skill_id,
"skill_name": "doc",
"event_params": {
"product_client_id": originator().value,
"skill_scope": "user",
"repo_url": null,
"thread_id": "thread-1",
"invoke_type": "explicit",
"model_slug": "gpt-5"
}
}])
);
}
#[tokio::test]
async fn reducer_ingests_hook_run_fact() {
let mut reducer = AnalyticsReducer::default();

View file

@ -9,8 +9,6 @@ use crate::facts::AnalyticsJsonRpcError;
use crate::facts::CustomAnalyticsFact;
use crate::facts::HookRunFact;
use crate::facts::HookRunInput;
use crate::facts::SkillInvocation;
use crate::facts::SkillInvokedInput;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::TrackEventsContext;
use crate::facts::TurnResolvedConfigFact;
@ -75,22 +73,6 @@ impl AnalyticsEventsClient {
}
}
pub fn track_skill_invocations(
&self,
tracking: TrackEventsContext,
invocations: Vec<SkillInvocation>,
) {
if invocations.is_empty() {
return;
}
self.record_fact(AnalyticsFact::Custom(CustomAnalyticsFact::SkillInvoked(
SkillInvokedInput {
tracking,
invocations,
},
)));
}
pub fn track_initialize(
&self,
connection_id: u64,

View file

@ -8,7 +8,6 @@ use crate::facts::CompactionStatus;
use crate::facts::CompactionStrategy;
use crate::facts::CompactionTrigger;
use crate::facts::HookRunFact;
use crate::facts::InvocationType;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::ThreadInitializationMode;
use crate::facts::TrackEventsContext;
@ -48,7 +47,6 @@ pub(crate) struct TrackEventsRequest {
#[derive(Serialize)]
#[serde(untagged)]
pub(crate) enum TrackEventRequest {
SkillInvocation(SkillInvocationEventRequest),
ThreadInitialized(ThreadInitializedEvent),
GuardianReview(Box<GuardianReviewEventRequest>),
HookRun(CodexHookRunEventRequest),
@ -57,24 +55,6 @@ pub(crate) enum TrackEventRequest {
TurnSteer(CodexTurnSteerEventRequest),
}
#[derive(Serialize)]
pub(crate) struct SkillInvocationEventRequest {
pub(crate) event_type: &'static str,
pub(crate) skill_id: String,
pub(crate) skill_name: String,
pub(crate) event_params: SkillInvocationEventParams,
}
#[derive(Serialize)]
pub(crate) struct SkillInvocationEventParams {
pub(crate) product_client_id: Option<String>,
pub(crate) skill_scope: Option<String>,
pub(crate) repo_url: Option<String>,
pub(crate) thread_id: Option<String>,
pub(crate) invoke_type: Option<InvocationType>,
pub(crate) model_slug: Option<String>,
}
#[derive(Clone, Serialize)]
pub(crate) struct CodexAppServerClientMetadata {
pub(crate) product_client_id: String,

View file

@ -19,11 +19,9 @@ use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookSource;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SkillScope;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::TokenUsage;
use serde::Serialize;
use std::path::PathBuf;
#[derive(Clone)]
pub struct TrackEventsContext {
@ -164,21 +162,6 @@ impl From<InputError> for TurnSteerRejectionReason {
}
}
#[derive(Clone, Debug)]
pub struct SkillInvocation {
pub skill_name: String,
pub skill_scope: SkillScope,
pub skill_path: PathBuf,
pub invocation_type: InvocationType,
}
#[derive(Clone, Copy, Debug, Serialize)]
#[serde(rename_all = "lowercase")]
pub enum InvocationType {
Explicit,
Implicit,
}
#[derive(Clone)]
pub struct SubAgentThreadStartedInput {
pub thread_id: String,
@ -291,15 +274,9 @@ pub(crate) enum CustomAnalyticsFact {
GuardianReview(Box<GuardianReviewEventParams>),
TurnResolvedConfig(Box<TurnResolvedConfigFact>),
TurnTokenUsage(Box<TurnTokenUsageFact>),
SkillInvoked(SkillInvokedInput),
HookRun(HookRunInput),
}
pub(crate) struct SkillInvokedInput {
pub tracking: TrackEventsContext,
pub invocations: Vec<SkillInvocation>,
}
pub(crate) struct HookRunInput {
pub tracking: TrackEventsContext,
pub hook: HookRunFact,

View file

@ -28,8 +28,6 @@ pub use facts::CompactionStrategy;
pub use facts::CompactionTrigger;
pub use facts::HookRunFact;
pub use facts::InputError;
pub use facts::InvocationType;
pub use facts::SkillInvocation;
pub use facts::SubAgentThreadStartedInput;
pub use facts::ThreadInitializationMode;
pub use facts::TrackEventsContext;

View file

@ -10,8 +10,6 @@ use crate::events::CodexTurnSteerEventRequest;
use crate::events::GuardianReviewEventParams;
use crate::events::GuardianReviewEventPayload;
use crate::events::GuardianReviewEventRequest;
use crate::events::SkillInvocationEventParams;
use crate::events::SkillInvocationEventRequest;
use crate::events::ThreadInitializedEvent;
use crate::events::ThreadInitializedEventParams;
use crate::events::TrackEventRequest;
@ -25,7 +23,6 @@ use crate::facts::AnalyticsJsonRpcError;
use crate::facts::CodexCompactionEvent;
use crate::facts::CustomAnalyticsFact;
use crate::facts::HookRunInput;
use crate::facts::SkillInvokedInput;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::ThreadInitializationMode;
use crate::facts::TurnResolvedConfigFact;
@ -42,19 +39,13 @@ use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::TurnSteerResponse;
use codex_app_server_protocol::UserInput;
use codex_git_utils::collect_git_info;
use codex_git_utils::get_git_repo_root;
use codex_login::default_client::originator;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SkillScope;
use codex_protocol::protocol::TokenUsage;
use sha1::Digest;
use std::collections::HashMap;
use std::path::Path;
#[derive(Default)]
pub(crate) struct AnalyticsReducer {
@ -198,9 +189,6 @@ impl AnalyticsReducer {
CustomAnalyticsFact::TurnTokenUsage(input) => {
self.ingest_turn_token_usage(*input, out);
}
CustomAnalyticsFact::SkillInvoked(input) => {
self.ingest_skill_invoked(input, out).await;
}
CustomAnalyticsFact::HookRun(input) => {
self.ingest_hook_run(input, out);
}
@ -355,54 +343,6 @@ impl AnalyticsReducer {
self.maybe_emit_turn_event(&turn_id, out);
}
async fn ingest_skill_invoked(
&mut self,
input: SkillInvokedInput,
out: &mut Vec<TrackEventRequest>,
) {
let SkillInvokedInput {
tracking,
invocations,
} = input;
for invocation in invocations {
let skill_scope = match invocation.skill_scope {
SkillScope::User => "user",
SkillScope::Repo => "repo",
SkillScope::System => "system",
SkillScope::Admin => "admin",
};
let repo_root = get_git_repo_root(invocation.skill_path.as_path());
let repo_url = if let Some(root) = repo_root.as_ref() {
collect_git_info(root)
.await
.and_then(|info| info.repository_url)
} else {
None
};
let skill_id = skill_id_for_local_skill(
repo_url.as_deref(),
repo_root.as_deref(),
invocation.skill_path.as_path(),
invocation.skill_name.as_str(),
);
out.push(TrackEventRequest::SkillInvocation(
SkillInvocationEventRequest {
event_type: "skill_invocation",
skill_id,
skill_name: invocation.skill_name.clone(),
event_params: SkillInvocationEventParams {
thread_id: Some(tracking.thread_id.clone()),
invoke_type: Some(invocation.invocation_type),
model_slug: Some(tracking.model_slug.clone()),
product_client_id: Some(originator().value),
repo_url,
skill_scope: Some(skill_scope.to_string()),
},
},
));
}
}
fn ingest_hook_run(&mut self, input: HookRunInput, out: &mut Vec<TrackEventRequest>) {
let HookRunInput { tracking, hook } = input;
out.push(TrackEventRequest::HookRun(CodexHookRunEventRequest {
@ -937,45 +877,3 @@ fn rejection_reason_from_error_type(
AnalyticsJsonRpcError::Input(error) => Some(error.into()),
}
}
pub(crate) fn skill_id_for_local_skill(
repo_url: Option<&str>,
repo_root: Option<&Path>,
skill_path: &Path,
skill_name: &str,
) -> String {
let path = normalize_path_for_skill_id(repo_url, repo_root, skill_path);
let prefix = if let Some(url) = repo_url {
format!("repo_{url}")
} else {
"personal".to_string()
};
let raw_id = format!("{prefix}_{path}_{skill_name}");
let mut hasher = sha1::Sha1::new();
sha1::Digest::update(&mut hasher, raw_id.as_bytes());
format!("{:x}", sha1::Digest::finalize(hasher))
}
/// Returns a normalized path for skill ID construction.
///
/// - Repo-scoped skills use a path relative to the repo root.
/// - User/admin/system skills use an absolute path.
pub(crate) fn normalize_path_for_skill_id(
repo_url: Option<&str>,
repo_root: Option<&Path>,
skill_path: &Path,
) -> String {
let resolved_path =
std::fs::canonicalize(skill_path).unwrap_or_else(|_| skill_path.to_path_buf());
match (repo_url, repo_root) {
(Some(_), Some(root)) => {
let resolved_root = std::fs::canonicalize(root).unwrap_or_else(|_| root.to_path_buf());
resolved_path
.strip_prefix(&resolved_root)
.unwrap_or(resolved_path.as_path())
.to_string_lossy()
.replace('\\', "/")
}
_ => resolved_path.to_string_lossy().replace('\\', "/"),
}
}

View file

@ -11,14 +11,12 @@ pub use mcp::McpOAuthScopesSource;
pub use mcp::McpServerStatusSnapshot;
pub use mcp::McpSnapshotDetail;
pub use mcp::ResolvedMcpOAuthScopes;
pub use mcp::canonical_mcp_server_key;
pub use mcp::collect_mcp_server_status_snapshot;
pub use mcp::collect_mcp_server_status_snapshot_with_detail;
pub use mcp::collect_mcp_snapshot;
pub use mcp::collect_mcp_snapshot_from_manager;
pub use mcp::collect_mcp_snapshot_from_manager_with_detail;
pub use mcp::collect_mcp_snapshot_with_detail;
pub use mcp::collect_missing_mcp_dependencies;
pub use mcp::compute_auth_statuses;
pub use mcp::configured_mcp_servers;
pub use mcp::discover_supported_scopes;

View file

@ -1,5 +1,4 @@
pub(crate) mod auth;
mod skill_dependencies;
pub use auth::McpAuthStatusEntry;
pub use auth::McpOAuthLoginConfig;
pub use auth::McpOAuthLoginSupport;
@ -10,8 +9,6 @@ pub use auth::discover_supported_scopes;
pub use auth::oauth_login_support;
pub use auth::resolve_oauth_scopes;
pub use auth::should_retry_without_scopes;
pub use skill_dependencies::canonical_mcp_server_key;
pub use skill_dependencies::collect_missing_mcp_dependencies;
use std::collections::HashMap;
use std::path::PathBuf;
@ -111,8 +108,6 @@ pub struct McpConfig {
pub mcp_oauth_callback_port: Option<u16>,
/// Optional OAuth redirect URI override for MCP login.
pub mcp_oauth_callback_url: Option<String>,
/// Whether skill MCP dependency installation prompts are enabled.
pub skill_mcp_dependency_install_enabled: bool,
/// Approval policy used for MCP tool calls and MCP elicitation requests.
pub approval_policy: Constrained<AskForApproval>,
/// Optional path to `codex-linux-sandbox` for sandboxed MCP tool execution.

View file

@ -13,7 +13,6 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::default(),
mcp_oauth_callback_port: None,
mcp_oauth_callback_url: None,
skill_mcp_dependency_install_enabled: true,
approval_policy: Constrained::allow_any(AskForApproval::OnFailure),
codex_linux_sandbox_exe: None,
use_legacy_landlock: false,

View file

@ -1,172 +0,0 @@
use std::collections::HashMap;
use std::collections::HashSet;
use codex_config::McpServerConfig;
use codex_config::McpServerTransportConfig;
use codex_protocol::protocol::SkillMetadata;
use codex_protocol::protocol::SkillToolDependency;
use tracing::warn;
pub fn collect_missing_mcp_dependencies(
mentioned_skills: &[SkillMetadata],
installed: &HashMap<String, McpServerConfig>,
) -> HashMap<String, McpServerConfig> {
let mut missing = HashMap::new();
let installed_keys: HashSet<String> = installed
.iter()
.map(|(name, config)| canonical_mcp_server_key(name, config))
.collect();
let mut seen_canonical_keys = HashSet::new();
for skill in mentioned_skills {
let Some(dependencies) = skill.dependencies.as_ref() else {
continue;
};
for tool in &dependencies.tools {
if !tool.r#type.eq_ignore_ascii_case("mcp") {
continue;
}
let dependency_key = match canonical_mcp_dependency_key(tool) {
Ok(key) => key,
Err(err) => {
let dependency = tool.value.as_str();
let skill_name = skill.name.as_str();
warn!(
"unable to auto-install MCP dependency {dependency} for skill {skill_name}: {err}",
);
continue;
}
};
if installed_keys.contains(&dependency_key)
|| seen_canonical_keys.contains(&dependency_key)
{
continue;
}
let config = match mcp_dependency_to_server_config(tool) {
Ok(config) => config,
Err(err) => {
let dependency = dependency_key.as_str();
let skill_name = skill.name.as_str();
warn!(
"unable to auto-install MCP dependency {dependency} for skill {skill_name}: {err}",
);
continue;
}
};
missing.insert(tool.value.clone(), config);
seen_canonical_keys.insert(dependency_key);
}
}
missing
}
fn canonical_mcp_key(transport: &str, identifier: &str, fallback: &str) -> String {
let identifier = identifier.trim();
if identifier.is_empty() {
fallback.to_string()
} else {
format!("mcp__{transport}__{identifier}")
}
}
pub fn canonical_mcp_server_key(name: &str, config: &McpServerConfig) -> String {
match &config.transport {
McpServerTransportConfig::Stdio { command, .. } => {
canonical_mcp_key("stdio", command, name)
}
McpServerTransportConfig::StreamableHttp { url, .. } => {
canonical_mcp_key("streamable_http", url, name)
}
}
}
fn canonical_mcp_dependency_key(dependency: &SkillToolDependency) -> Result<String, String> {
let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
if transport.eq_ignore_ascii_case("streamable_http") {
let url = dependency
.url
.as_ref()
.ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
return Ok(canonical_mcp_key("streamable_http", url, &dependency.value));
}
if transport.eq_ignore_ascii_case("stdio") {
let command = dependency
.command
.as_ref()
.ok_or_else(|| "missing command for stdio dependency".to_string())?;
return Ok(canonical_mcp_key("stdio", command, &dependency.value));
}
Err(format!("unsupported transport {transport}"))
}
fn mcp_dependency_to_server_config(
dependency: &SkillToolDependency,
) -> Result<McpServerConfig, String> {
let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
if transport.eq_ignore_ascii_case("streamable_http") {
let url = dependency
.url
.as_ref()
.ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
return Ok(McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: url.clone(),
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
experimental_environment: None,
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
});
}
if transport.eq_ignore_ascii_case("stdio") {
let command = dependency
.command
.as_ref()
.ok_or_else(|| "missing command for stdio dependency".to_string())?;
return Ok(McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: command.clone(),
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
experimental_environment: None,
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
});
}
Err(format!("unsupported transport {transport}"))
}
#[cfg(test)]
#[path = "skill_dependencies_tests.rs"]
mod tests;

View file

@ -1,115 +0,0 @@
use super::*;
use codex_protocol::protocol::SkillDependencies;
use codex_protocol::protocol::SkillMetadata;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::test_support::PathBufExt as _;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
fn skill_with_tools(tools: Vec<SkillToolDependency>) -> SkillMetadata {
SkillMetadata {
name: "skill".to_string(),
description: "skill".to_string(),
short_description: None,
interface: None,
dependencies: Some(SkillDependencies { tools }),
path: test_path_buf("/tmp/skill").abs(),
scope: SkillScope::User,
enabled: true,
}
}
#[test]
fn collect_missing_respects_canonical_installed_key() {
let url = "https://example.com/mcp".to_string();
let skills = vec![skill_with_tools(vec![SkillToolDependency {
r#type: "mcp".to_string(),
value: "github".to_string(),
description: None,
transport: Some("streamable_http".to_string()),
command: None,
url: Some(url.clone()),
}])];
let installed = HashMap::from([(
"alias".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url,
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
experimental_environment: None,
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
)]);
assert_eq!(
collect_missing_mcp_dependencies(&skills, &installed),
HashMap::new()
);
}
#[test]
fn collect_missing_dedupes_by_canonical_key_but_preserves_original_name() {
let url = "https://example.com/one".to_string();
let skills = vec![skill_with_tools(vec![
SkillToolDependency {
r#type: "mcp".to_string(),
value: "alias-one".to_string(),
description: None,
transport: Some("streamable_http".to_string()),
command: None,
url: Some(url.clone()),
},
SkillToolDependency {
r#type: "mcp".to_string(),
value: "alias-two".to_string(),
description: None,
transport: Some("streamable_http".to_string()),
command: None,
url: Some(url.clone()),
},
])];
let expected = HashMap::from([(
"alias-one".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url,
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
experimental_environment: None,
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
)]);
assert_eq!(
collect_missing_mcp_dependencies(&skills, &HashMap::new()),
expected
);
}

View file

@ -18,7 +18,6 @@ use crate::types::OAuthCredentialsStoreMode;
use crate::types::OtelConfigToml;
use crate::types::SandboxWorkspaceWrite;
use crate::types::ShellEnvironmentPolicyToml;
use crate::types::SkillsConfig;
use crate::types::Tui;
use crate::types::UriBasedFileOpener;
use crate::types::WindowsToml;
@ -321,9 +320,6 @@ pub struct ConfigToml {
/// Memories subsystem settings.
pub memories: Option<MemoriesToml>,
/// User-level skill config entries keyed by SKILL.md path.
pub skills: Option<SkillsConfig>,
/// Centralized feature flags (new). Prefer this over individual toggles.
#[serde(default)]
// Injects known feature keys into the schema and forbids unknown keys.

View file

@ -16,7 +16,6 @@ mod project_root_markers;
mod requirements_exec_policy;
pub mod schema;
pub mod shell_environment;
mod skills_config;
mod state;
mod thread_config;
pub mod types;
@ -82,9 +81,6 @@ pub use requirements_exec_policy::RequirementsExecPolicyParseError;
pub use requirements_exec_policy::RequirementsExecPolicyPatternTokenToml;
pub use requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml;
pub use requirements_exec_policy::RequirementsExecPolicyToml;
pub use skills_config::BundledSkillsConfig;
pub use skills_config::SkillConfig;
pub use skills_config::SkillsConfig;
pub use state::ConfigLayerEntry;
pub use state::ConfigLayerStack;
pub use state::ConfigLayerStackOrdering;

View file

@ -1,57 +0,0 @@
//! Skill-related configuration types shared across crates.
use codex_utils_absolute_path::AbsolutePathBuf;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
const fn default_enabled() -> bool {
true
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SkillConfig {
/// Path-based selector.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub path: Option<AbsolutePathBuf>,
/// Name-based selector.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
pub enabled: bool,
}
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SkillsConfig {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub bundled: Option<BundledSkillsConfig>,
/// Whether turns receive the automatic skills instructions block.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub include_instructions: Option<bool>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub config: Vec<SkillConfig>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct BundledSkillsConfig {
#[serde(default = "default_enabled")]
pub enabled: bool,
}
impl Default for BundledSkillsConfig {
fn default() -> Self {
Self { enabled: true }
}
}
impl TryFrom<toml::Value> for SkillsConfig {
type Error = toml::de::Error;
fn try_from(value: toml::Value) -> Result<Self, Self::Error> {
SkillsConfig::deserialize(value)
}
}

View file

@ -517,10 +517,6 @@ pub struct Notice {
pub external_config_migration_prompts: ExternalConfigMigrationPrompts,
}
pub use crate::skills_config::BundledSkillsConfig;
pub use crate::skills_config::SkillConfig;
pub use crate::skills_config::SkillsConfig;
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SandboxWorkspaceWrite {

View file

@ -1,47 +1,6 @@
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn deserialize_skill_config_with_name_selector() {
let cfg: SkillConfig = toml::from_str(
r#"
name = "github:yeet"
enabled = false
"#,
)
.expect("should deserialize skill config with name selector");
assert_eq!(cfg.name.as_deref(), Some("github:yeet"));
assert_eq!(cfg.path, None);
assert!(!cfg.enabled);
}
#[test]
fn deserialize_skill_config_with_path_selector() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_path = tempdir.path().join("skills").join("demo").join("SKILL.md");
let cfg: SkillConfig = toml::from_str(&format!(
r#"
path = {path:?}
enabled = false
"#,
path = skill_path.display().to_string(),
))
.expect("should deserialize skill config with path selector");
assert_eq!(
cfg,
SkillConfig {
path: Some(
AbsolutePathBuf::from_absolute_path(&skill_path)
.expect("skill path should be absolute"),
),
name: None,
enabled: false,
}
);
}
#[test]
fn memories_config_clamps_count_limits_to_nonzero_values() {
let config = MemoriesConfig::from(MemoriesToml {

View file

@ -185,16 +185,6 @@
},
"type": "object"
},
"BundledSkillsConfig": {
"additionalProperties": false,
"properties": {
"enabled": {
"default": true,
"type": "boolean"
}
},
"type": "object"
},
"ConfigProfile": {
"additionalProperties": false,
"description": "Collection of common configuration options that a user can define as a unit in `config.toml`.",
@ -1720,49 +1710,6 @@
},
"type": "object"
},
"SkillConfig": {
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"name": {
"description": "Name-based selector.",
"type": "string"
},
"path": {
"allOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
}
],
"description": "Path-based selector."
}
},
"required": [
"enabled"
],
"type": "object"
},
"SkillsConfig": {
"additionalProperties": false,
"properties": {
"bundled": {
"$ref": "#/definitions/BundledSkillsConfig"
},
"config": {
"items": {
"$ref": "#/definitions/SkillConfig"
},
"type": "array"
},
"include_instructions": {
"description": "Whether turns receive the automatic skills instructions block.",
"type": "boolean"
}
},
"type": "object"
},
"ToolsToml": {
"additionalProperties": false,
"properties": {
@ -2658,14 +2605,6 @@
"description": "When set to `true`, `AgentReasoningRawContentEvent` events will be shown in the UI/output. Defaults to `false`.",
"type": "boolean"
},
"skills": {
"allOf": [
{
"$ref": "#/definitions/SkillsConfig"
}
],
"description": "User-level skill config entries keyed by SKILL.md path."
},
"sqlite_home": {
"allOf": [
{

View file

@ -28,7 +28,6 @@ use codex_config::permissions_toml::PermissionProfileToml;
use codex_config::permissions_toml::PermissionsToml;
use codex_config::profile_toml::ConfigProfile;
use codex_config::types::ApprovalsReviewer;
use codex_config::types::BundledSkillsConfig;
use codex_config::types::FeedbackConfigToml;
use codex_config::types::HistoryPersistence;
use codex_config::types::McpServerEnvVar;
@ -42,7 +41,6 @@ use codex_config::types::NotificationCondition;
use codex_config::types::NotificationMethod;
use codex_config::types::Notifications;
use codex_config::types::SandboxWorkspaceWrite;
use codex_config::types::SkillsConfig;
use codex_config::types::Tui;
use codex_config::types::TuiNotificationSettings;
use codex_exec_server::LOCAL_FS;
@ -291,29 +289,6 @@ consolidation_model = "gpt-5.2"
);
}
#[test]
fn parses_bundled_skills_config() {
let cfg: ConfigToml = toml::from_str(
r#"
[skills]
include_instructions = false
[skills.bundled]
enabled = false
"#,
)
.expect("TOML deserialization should succeed");
assert_eq!(
cfg.skills,
Some(SkillsConfig {
bundled: Some(BundledSkillsConfig { enabled: false }),
include_instructions: Some(false),
config: Vec::new(),
})
);
}
#[test]
fn tools_web_search_true_deserializes_to_none() {
let cfg: ConfigToml = toml::from_str(
@ -2601,9 +2576,6 @@ async fn to_mcp_config_quarantines_codex_product_surfaces() -> std::io::Result<(
)
.await?;
let mcp_config = config.to_mcp_config();
assert!(!mcp_config.skill_mcp_dependency_install_enabled);
assert_eq!(config.to_mcp_config().configured_mcp_servers.len(), 0);
assert_eq!(config.to_mcp_config().configured_mcp_servers.len(), 0);
Ok(())
@ -5225,7 +5197,6 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
developer_instructions: None,
guardian_policy_config: None,
include_permissions_instructions: true,
include_skill_instructions: true,
include_environment_context: true,
compact_prompt: None,
commit_attribution: None,
@ -5374,7 +5345,6 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
developer_instructions: None,
guardian_policy_config: None,
include_permissions_instructions: true,
include_skill_instructions: true,
include_environment_context: true,
compact_prompt: None,
commit_attribution: None,
@ -5521,7 +5491,6 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
developer_instructions: None,
guardian_policy_config: None,
include_permissions_instructions: true,
include_skill_instructions: true,
include_environment_context: true,
compact_prompt: None,
commit_attribution: None,
@ -5653,7 +5622,6 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
developer_instructions: None,
guardian_policy_config: None,
include_permissions_instructions: true,
include_skill_instructions: true,
include_environment_context: true,
compact_prompt: None,
commit_attribution: None,
@ -6629,9 +6597,6 @@ async fn prompt_instruction_blocks_can_be_disabled_from_config_and_profiles() ->
include_environment_context = false
profile = "chatty"
[skills]
include_instructions = false
[profiles.chatty]
include_permissions_instructions = true
include_environment_context = true
@ -6645,7 +6610,6 @@ include_environment_context = true
.await?;
assert!(config.include_permissions_instructions);
assert!(!config.include_skill_instructions);
assert!(config.include_environment_context);
Ok(())
}

View file

@ -13,7 +13,6 @@ use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
use tokio::task;
use toml_edit::ArrayOfTables;
use toml_edit::DocumentMut;
use toml_edit::Item as TomlItem;
use toml_edit::Table as TomlTable;
@ -55,10 +54,6 @@ pub enum ConfigEdit {
RecordModelMigrationSeen { from: String, to: String },
/// Replace the entire `[mcp_servers]` table.
ReplaceMcpServers(BTreeMap<String, McpServerConfig>),
/// Set or clear a skill config entry under `[[skills.config]]` by path.
SetSkillConfig { path: PathBuf, enabled: bool },
/// Set or clear a skill config entry under `[[skills.config]]` by name.
SetSkillConfigByName { name: String, enabled: bool },
/// Set trust_level under `[projects."<path>"]`,
/// migrating inline tables to explicit tables.
SetProjectTrustLevel { path: PathBuf, level: TrustLevel },
@ -71,12 +66,6 @@ pub enum ConfigEdit {
ClearPath { segments: Vec<String> },
}
#[derive(Clone, Debug, PartialEq, Eq)]
enum SkillConfigSelector {
Name(String),
Path(PathBuf),
}
/// Produces a config edit that sets `[tui].theme = "<name>"`.
pub fn syntax_theme_edit(name: &str) -> ConfigEdit {
ConfigEdit::SetPath {
@ -506,12 +495,6 @@ impl ConfigDocument {
value(*acknowledged),
)),
ConfigEdit::ReplaceMcpServers(servers) => Ok(self.replace_mcp_servers(servers)),
ConfigEdit::SetSkillConfig { path, enabled } => {
Ok(self.set_skill_config(SkillConfigSelector::Path(path.clone()), *enabled))
}
ConfigEdit::SetSkillConfigByName { name, enabled } => {
Ok(self.set_skill_config(SkillConfigSelector::Name(name.clone()), *enabled))
}
ConfigEdit::SetPath { segments, value } => Ok(self.insert(segments, value.clone())),
ConfigEdit::ClearPath { segments } => Ok(self.clear_owned(segments)),
ConfigEdit::SetProjectTrustLevel { path, level } => {
@ -601,117 +584,6 @@ impl ConfigDocument {
true
}
fn set_skill_config(&mut self, selector: SkillConfigSelector, enabled: bool) -> bool {
let selector = match selector {
SkillConfigSelector::Name(name) => SkillConfigSelector::Name(name.trim().to_string()),
SkillConfigSelector::Path(path) => {
SkillConfigSelector::Path(PathBuf::from(normalize_skill_config_path(&path)))
}
};
if matches!(&selector, SkillConfigSelector::Name(name) if name.is_empty()) {
return false;
}
let mut remove_skills_table = false;
let mut mutated = false;
{
let root = self.doc.as_table_mut();
let skills_item = match root.get_mut("skills") {
Some(item) => item,
None => {
if enabled {
return false;
}
root.insert(
"skills",
TomlItem::Table(document_helpers::new_implicit_table()),
);
let Some(item) = root.get_mut("skills") else {
return false;
};
item
}
};
if document_helpers::ensure_table_for_write(skills_item).is_none() {
if enabled {
return false;
}
*skills_item = TomlItem::Table(document_helpers::new_implicit_table());
}
let Some(skills_table) = skills_item.as_table_mut() else {
return false;
};
let config_item = match skills_table.get_mut("config") {
Some(item) => item,
None => {
if enabled {
return false;
}
skills_table.insert("config", TomlItem::ArrayOfTables(ArrayOfTables::new()));
let Some(item) = skills_table.get_mut("config") else {
return false;
};
item
}
};
if !matches!(config_item, TomlItem::ArrayOfTables(_)) {
if enabled {
return false;
}
*config_item = TomlItem::ArrayOfTables(ArrayOfTables::new());
}
let TomlItem::ArrayOfTables(overrides) = config_item else {
return false;
};
let existing_index = overrides.iter().enumerate().find_map(|(idx, table)| {
skill_config_selector_from_table(table)
.filter(|value| value == &selector)
.map(|_| idx)
});
if enabled {
if let Some(index) = existing_index {
overrides.remove(index);
mutated = true;
if overrides.is_empty() {
skills_table.remove("config");
if skills_table.is_empty() {
remove_skills_table = true;
}
}
}
} else if let Some(index) = existing_index {
for (idx, table) in overrides.iter_mut().enumerate() {
if idx == index {
write_skill_config_selector(table, &selector);
table["enabled"] = value(false);
mutated = true;
break;
}
}
} else {
let mut entry = TomlTable::new();
entry.set_implicit(false);
write_skill_config_selector(&mut entry, &selector);
entry["enabled"] = value(false);
overrides.push(entry);
mutated = true;
}
}
if remove_skills_table {
let root = self.doc.as_table_mut();
root.remove("skills");
}
mutated
}
fn scoped_segments(&self, scope: Scope, segments: &[&str]) -> Vec<String> {
let resolved: Vec<String> = segments
.iter()
@ -819,45 +691,6 @@ impl ConfigDocument {
}
}
fn normalize_skill_config_path(path: &Path) -> String {
dunce::canonicalize(path)
.unwrap_or_else(|_| path.to_path_buf())
.to_string_lossy()
.to_string()
}
fn skill_config_selector_from_table(table: &TomlTable) -> Option<SkillConfigSelector> {
let path = table
.get("path")
.and_then(|item| item.as_str())
.map(Path::new)
.map(|path| SkillConfigSelector::Path(PathBuf::from(normalize_skill_config_path(path))));
let name = table
.get("name")
.and_then(|item| item.as_str())
.map(str::trim)
.filter(|name| !name.is_empty())
.map(|name| SkillConfigSelector::Name(name.to_string()));
match (path, name) {
(Some(selector), None) | (None, Some(selector)) => Some(selector),
_ => None,
}
}
fn write_skill_config_selector(table: &mut TomlTable, selector: &SkillConfigSelector) {
match selector {
SkillConfigSelector::Name(name) => {
table.remove("path");
table["name"] = value(name.clone());
}
SkillConfigSelector::Path(path) => {
table.remove("name");
table["path"] = value(path.to_string_lossy().to_string());
}
}
}
/// Persist edits using a blocking strategy.
pub fn apply_blocking(
codex_home: &Path,

View file

@ -66,73 +66,6 @@ gpt-foo = 4
assert_eq!(contents, expected);
}
#[test]
fn set_skill_config_writes_disabled_entry() {
let tmp = tempdir().expect("tmpdir");
let codex_home = tmp.path();
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::SetSkillConfig {
path: PathBuf::from("/tmp/skills/demo/SKILL.md"),
enabled: false,
}])
.apply_blocking()
.expect("persist");
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
let expected = r#"[[skills.config]]
path = "/tmp/skills/demo/SKILL.md"
enabled = false
"#;
assert_eq!(contents, expected);
}
#[test]
fn set_skill_config_removes_entry_when_enabled() {
let tmp = tempdir().expect("tmpdir");
let codex_home = tmp.path();
std::fs::write(
codex_home.join(CONFIG_TOML_FILE),
r#"[[skills.config]]
path = "/tmp/skills/demo/SKILL.md"
enabled = false
"#,
)
.expect("seed config");
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::SetSkillConfig {
path: PathBuf::from("/tmp/skills/demo/SKILL.md"),
enabled: true,
}])
.apply_blocking()
.expect("persist");
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
assert_eq!(contents, "");
}
#[test]
fn set_skill_config_writes_name_selector_entry() {
let tmp = tempdir().expect("tmpdir");
let codex_home = tmp.path();
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::SetSkillConfigByName {
name: "github:yeet".to_string(),
enabled: false,
}])
.apply_blocking()
.expect("persist");
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
let expected = r#"[[skills.config]]
name = "github:yeet"
enabled = false
"#;
assert_eq!(contents, expected);
}
#[test]
fn blocking_set_model_preserves_inline_table_contents() {
let tmp = tempdir().expect("tmpdir");

View file

@ -299,9 +299,6 @@ pub struct Config {
/// Whether to inject the `<permissions instructions>` developer block.
pub include_permissions_instructions: bool,
/// Whether to inject the `<skills_instructions>` developer block.
pub include_skill_instructions: bool,
/// Whether to inject the `<environment_context>` user block.
pub include_environment_context: bool,
@ -805,7 +802,6 @@ impl Config {
mcp_oauth_credentials_store_mode: self.mcp_oauth_credentials_store_mode,
mcp_oauth_callback_port: self.mcp_oauth_callback_port,
mcp_oauth_callback_url: self.mcp_oauth_callback_url.clone(),
skill_mcp_dependency_install_enabled: false,
approval_policy: self.permissions.approval_policy.clone(),
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
use_legacy_landlock: self.features.use_legacy_landlock(),
@ -2003,11 +1999,6 @@ impl Config {
.include_permissions_instructions
.or(cfg.include_permissions_instructions)
.unwrap_or(true);
let include_skill_instructions = cfg
.skills
.as_ref()
.and_then(|skills| skills.include_instructions)
.unwrap_or(true);
let include_environment_context = config_profile
.include_environment_context
.or(cfg.include_environment_context)
@ -2228,7 +2219,6 @@ impl Config {
compact_prompt,
commit_attribution,
include_permissions_instructions,
include_skill_instructions,
include_environment_context,
// The config.toml omits "_mode" because it's a config file. However, "_mode"
// is important in code to differentiate the mode from the store implementation.

View file

@ -836,7 +836,6 @@ pub(crate) fn build_guardian_review_session_config(
let mut guardian_config = parent_config.clone();
guardian_config.model = Some(active_model.to_string());
guardian_config.model_reasoning_effort = reasoning_effort;
guardian_config.include_skill_instructions = false;
guardian_config.base_instructions = Some(
parent_config
.guardian_policy_config
@ -997,22 +996,6 @@ mod tests {
assert!(!guardian_config.features.enabled(Feature::CodexHooks));
}
#[tokio::test]
async fn guardian_review_session_config_disables_skill_instructions() {
let mut parent_config = crate::config::test_config().await;
parent_config.include_skill_instructions = true;
let guardian_config = build_guardian_review_session_config(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect("guardian config");
assert!(!guardian_config.include_skill_instructions);
}
#[tokio::test(flavor = "current_thread")]
async fn run_before_review_deadline_times_out_before_future_completes() {
let outcome = run_before_review_deadline(

View file

@ -85,7 +85,6 @@ mod search_tool;
mod shell_command;
mod shell_serialization;
mod shell_snapshot;
mod skill_approval;
mod spawn_agent_description;
mod sqlite_state;
mod stream_error_allows_next_turn;

View file

@ -1,281 +0,0 @@
#![allow(clippy::unwrap_used)]
#![cfg(unix)]
use anyhow::Result;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::mount_function_call_agent_response;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::test_codex::TestCodex;
use core_test_support::wait_for_event;
use core_test_support::wait_for_event_match;
use core_test_support::zsh_fork::build_zsh_fork_test;
use core_test_support::zsh_fork::restrictive_workspace_write_policy;
use core_test_support::zsh_fork::zsh_fork_runtime;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
fn write_skill_metadata(home: &Path, name: &str, contents: &str) -> Result<()> {
let metadata_dir = home.join("skills").join(name).join("agents");
fs::create_dir_all(&metadata_dir)?;
fs::write(metadata_dir.join("openai.yaml"), contents)?;
Ok(())
}
fn shell_command_arguments(command: &str) -> Result<String> {
Ok(serde_json::to_string(&serde_json::json!({
"command": command,
"timeout_ms": 500,
}))?)
}
async fn submit_turn_with_policies(
test: &TestCodex,
prompt: &str,
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
test.codex
.submit(Op::UserTurn {
environments: None,
items: vec![UserInput::Text {
text: prompt.to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd_path().to_path_buf(),
approval_policy,
approvals_reviewer: None,
sandbox_policy,
model: test.session_configured.model.clone(),
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
})
.await?;
Ok(())
}
#[cfg(unix)]
fn write_skill_with_shell_script_contents(
home: &Path,
name: &str,
script_name: &str,
script_contents: &str,
) -> Result<PathBuf> {
use std::os::unix::fs::PermissionsExt;
let skill_dir = home.join("skills").join(name);
let scripts_dir = skill_dir.join("scripts");
fs::create_dir_all(&scripts_dir)?;
fs::write(
skill_dir.join("SKILL.md"),
format!(
r#"---
name: {name}
description: {name} skill
---
"#
),
)?;
let script_path = scripts_dir.join(script_name);
fs::write(&script_path, script_contents)?;
let mut permissions = fs::metadata(&script_path)?.permissions();
permissions.set_mode(0o755);
fs::set_permissions(&script_path, permissions)?;
Ok(script_path)
}
fn skill_script_command(test: &TestCodex, script_name: &str) -> Result<String> {
let script_path = fs::canonicalize(
test.codex_home_path()
.join("skills/mbolin-test-skill/scripts")
.join(script_name),
)?;
Ok(shlex::try_join([script_path.to_string_lossy().as_ref()])?)
}
async fn wait_for_exec_approval_request(test: &TestCodex) -> Option<ExecApprovalRequestEvent> {
wait_for_event_match(test.codex.as_ref(), |event| match event {
EventMsg::ExecApprovalRequest(request) => Some(Some(request.clone())),
EventMsg::TurnComplete(_) => Some(None),
_ => None,
})
.await
}
async fn wait_for_turn_complete(test: &TestCodex) {
wait_for_event(test.codex.as_ref(), |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
}
fn output_shows_sandbox_denial(output: &str) -> bool {
output.contains("Permission denied")
|| output.contains("Operation not permitted")
|| output.contains("Read-only file system")
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn shell_zsh_fork_skill_scripts_ignore_declared_permissions() -> Result<()> {
skip_if_no_network!(Ok(()));
let Some(runtime) = zsh_fork_runtime("zsh-fork skill script ignores permissions test")? else {
return Ok(());
};
let approval_policy = AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: true,
skill_approval: false,
request_permissions: true,
mcp_elicitations: true,
});
let workspace_write_policy = restrictive_workspace_write_policy();
let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?;
let allowed_dir = outside_dir.path().join("allowed-output");
fs::create_dir_all(&allowed_dir)?;
let allowed_path = allowed_dir.join("allowed.txt");
let allowed_path_quoted = shlex::try_join([allowed_path.to_string_lossy().as_ref()])?;
let script_contents = format!(
"#!/bin/sh\nprintf '%s' allowed > {allowed_path_quoted}\nif [ -f {allowed_path_quoted} ]; then cat {allowed_path_quoted}; fi\n"
);
let permissions_yaml = format!(
"permissions:\n file_system:\n write:\n - \"{}\"\n",
allowed_dir.display()
);
let server = start_mock_server().await;
let allowed_path_for_hook = allowed_path.clone();
let script_contents_for_hook = script_contents.clone();
let test = build_zsh_fork_test(
&server,
runtime,
approval_policy,
workspace_write_policy.clone(),
move |home| {
let _ = fs::remove_file(&allowed_path_for_hook);
write_skill_with_shell_script_contents(
home,
"mbolin-test-skill",
"sandboxed.sh",
&script_contents_for_hook,
)
.unwrap();
write_skill_metadata(home, "mbolin-test-skill", &permissions_yaml).unwrap();
},
)
.await?;
let command = skill_script_command(&test, "sandboxed.sh")?;
let call_id = "zsh-fork-skill-script-ignores-permissions";
let arguments = shell_command_arguments(&command)?;
let mocks =
mount_function_call_agent_response(&server, call_id, &arguments, "shell_command").await;
submit_turn_with_policies(
&test,
"use $mbolin-test-skill",
approval_policy,
workspace_write_policy,
)
.await?;
let approval = wait_for_exec_approval_request(&test).await;
assert!(
approval.is_none(),
"expected skill script execution to skip the removed skill approval path"
);
wait_for_turn_complete(&test).await;
let call_output = mocks
.completion
.single_request()
.function_call_output(call_id);
let output = call_output["output"].as_str().unwrap_or_default();
assert!(
!output.contains("Execution denied: Execution forbidden by policy"),
"skill script should now be governed by the turn sandbox, not the removed skill approval gate: {output:?}"
);
assert!(
output_shows_sandbox_denial(output) || !output.contains("allowed"),
"expected the turn sandbox to block the out-of-workspace write, got output: {output:?}"
);
assert!(
!allowed_path.exists(),
"declared skill permissions should not widen script execution beyond the turn sandbox"
);
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn shell_zsh_fork_still_enforces_workspace_write_sandbox() -> Result<()> {
skip_if_no_network!(Ok(()));
let Some(runtime) = zsh_fork_runtime("zsh-fork workspace sandbox test")? else {
return Ok(());
};
let server = start_mock_server().await;
let tool_call_id = "zsh-fork-workspace-write-deny";
let outside_path = "/tmp/codex-zsh-fork-workspace-write-deny.txt";
let workspace_write_policy = restrictive_workspace_write_policy();
let _ = fs::remove_file(outside_path);
let test = build_zsh_fork_test(
&server,
runtime,
AskForApproval::Never,
workspace_write_policy.clone(),
move |_| {
let _ = fs::remove_file(outside_path);
},
)
.await?;
let command = format!("touch {outside_path}");
let arguments = shell_command_arguments(&command)?;
let mocks =
mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command")
.await;
submit_turn_with_policies(
&test,
"write outside workspace with zsh fork",
AskForApproval::Never,
workspace_write_policy,
)
.await?;
wait_for_turn_complete(&test).await;
let call_output = mocks
.completion
.single_request()
.function_call_output(tool_call_id);
let output = call_output["output"].as_str().unwrap_or_default();
assert!(
output_shows_sandbox_denial(output),
"expected sandbox denial, got output: {output:?}"
);
assert!(
!Path::new(outside_path).exists(),
"command should not write outside workspace under WorkspaceWrite policy"
);
Ok(())
}

View file

@ -19,8 +19,6 @@ use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::fs;
use std::path::Path;
use std::time::Duration;
use tokio::time::Instant;
use tokio::time::sleep;
@ -103,14 +101,6 @@ fn role_block(description: &str, role_name: &str) -> Option<String> {
Some(block.join("\n"))
}
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(())
}
async fn wait_for_spawned_thread_id(test: &TestCodex) -> Result<String> {
let deadline = Instant::now() + Duration::from_secs(2);
loop {
@ -520,102 +510,6 @@ async fn spawned_multi_agent_v2_child_receives_xml_tagged_developer_context() ->
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn skills_toggle_skips_instructions_for_parent_and_spawned_child() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let spawn_args = serde_json::to_string(&json!({
"message": CHILD_PROMPT,
"task_name": "worker",
}))?;
let spawn_turn = mount_sse_once_match(
&server,
|req: &wiremock::Request| body_contains(req, TURN_1_PROMPT),
sse(vec![
ev_response_created("resp-turn1-1"),
ev_function_call(SPAWN_CALL_ID, "spawn_agent", &spawn_args),
ev_completed("resp-turn1-1"),
]),
)
.await;
let _child_request_log = mount_sse_once_match(
&server,
|req: &wiremock::Request| {
body_contains(req, CHILD_PROMPT) && body_contains(req, "<spawned_agent_context>")
},
sse(vec![
ev_response_created("resp-child-1"),
ev_completed("resp-child-1"),
]),
)
.await;
let _turn1_followup = mount_sse_once_match(
&server,
|req: &wiremock::Request| {
body_contains(req, "function_call_output")
&& body_contains(req, "/root/worker")
&& !body_contains(req, "<spawned_agent_context>")
},
sse(vec![
ev_response_created("resp-turn1-2"),
ev_assistant_message("msg-turn1-2", "parent done"),
ev_completed("resp-turn1-2"),
]),
)
.await;
let mut builder = test_codex()
.with_pre_build_hook(|home| {
if let Err(err) = write_home_skill(home, "demo", "demo-skill", "demo skill") {
panic!("write home skill: {err}");
}
})
.with_config(|config| {
config
.features
.enable(Feature::Collab)
.expect("test config should allow feature update");
config
.features
.enable(Feature::MultiAgentV2)
.expect("test config should allow feature update");
config.include_skill_instructions = false;
});
let test = builder.build(&server).await?;
test.submit_turn(TURN_1_PROMPT).await?;
let parent_request = spawn_turn.single_request();
assert!(!parent_request.body_contains_text("<skills_instructions>"));
assert!(!parent_request.body_contains_text("demo-skill"));
let deadline = Instant::now() + Duration::from_secs(2);
let child_request = loop {
if let Some(request) = server
.received_requests()
.await
.unwrap_or_default()
.into_iter()
.find(|request| {
body_contains(request, CHILD_PROMPT)
&& body_contains(request, "<spawned_agent_context>")
})
{
break request;
}
if Instant::now() >= deadline {
anyhow::bail!("timed out waiting for spawned child request");
}
sleep(Duration::from_millis(10)).await;
};
assert!(!body_contains(&child_request, "<skills_instructions>"));
assert!(!body_contains(&child_request, "demo-skill"));
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() -> Result<()> {
skip_if_no_network!(Ok(()));

View file

@ -36,8 +36,3 @@ pub const STARTUP_PREWARM_DURATION_METRIC: &str = "codex.startup_prewarm.duratio
pub const STARTUP_PREWARM_AGE_AT_FIRST_TURN_METRIC: &str =
"codex.startup_prewarm.age_at_first_turn_ms";
pub const THREAD_STARTED_METRIC: &str = "codex.thread.started";
pub const THREAD_SKILLS_ENABLED_TOTAL_METRIC: &str = "codex.thread.skills.enabled_total";
pub const THREAD_SKILLS_KEPT_TOTAL_METRIC: &str = "codex.thread.skills.kept_total";
pub const THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC: &str =
"codex.thread.skills.description_truncated_chars";
pub const THREAD_SKILLS_TRUNCATED_METRIC: &str = "codex.thread.skills.truncated";

View file

@ -3586,76 +3586,6 @@ impl Product {
products.is_empty() || products.contains(self)
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
#[ts(rename_all = "snake_case")]
pub enum SkillScope {
User,
Repo,
System,
Admin,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
pub struct SkillMetadata {
pub name: String,
pub description: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
/// Legacy short_description from SKILL.md. Prefer SKILL.json interface.short_description.
pub short_description: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub interface: Option<SkillInterface>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub dependencies: Option<SkillDependencies>,
pub path: AbsolutePathBuf,
pub scope: SkillScope,
pub enabled: bool,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)]
pub struct SkillInterface {
#[ts(optional)]
pub display_name: Option<String>,
#[ts(optional)]
pub short_description: Option<String>,
#[ts(optional)]
pub icon_small: Option<AbsolutePathBuf>,
#[ts(optional)]
pub icon_large: Option<AbsolutePathBuf>,
#[ts(optional)]
pub brand_color: Option<String>,
#[ts(optional)]
pub default_prompt: Option<String>,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)]
pub struct SkillDependencies {
pub tools: Vec<SkillToolDependency>,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)]
pub struct SkillToolDependency {
#[serde(rename = "type")]
#[ts(rename = "type")]
pub r#type: String,
pub value: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub description: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub transport: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub command: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub url: Option<String>,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)]
pub struct SessionNetworkProxyRuntime {
pub http_addr: String,