mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
Merge branch 'main' into sticky-r
This commit is contained in:
@@ -25,6 +25,7 @@ use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot;
|
||||
use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow;
|
||||
use codex_protocol::protocol::SessionSource as CoreSessionSource;
|
||||
use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo;
|
||||
use codex_protocol::protocol::SkillInterface as CoreSkillInterface;
|
||||
use codex_protocol::protocol::SkillMetadata as CoreSkillMetadata;
|
||||
use codex_protocol::protocol::SkillScope as CoreSkillScope;
|
||||
use codex_protocol::protocol::TokenUsage as CoreTokenUsage;
|
||||
@@ -1252,13 +1253,35 @@ pub enum SkillScope {
|
||||
pub struct SkillMetadata {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
#[ts(optional)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
/// Legacy short_description from SKILL.md. Prefer SKILL.toml interface.short_description.
|
||||
pub short_description: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub path: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct SkillInterface {
|
||||
#[ts(optional)]
|
||||
pub display_name: Option<String>,
|
||||
#[ts(optional)]
|
||||
pub short_description: Option<String>,
|
||||
#[ts(optional)]
|
||||
pub icon_small: Option<PathBuf>,
|
||||
#[ts(optional)]
|
||||
pub icon_large: Option<PathBuf>,
|
||||
#[ts(optional)]
|
||||
pub brand_color: Option<String>,
|
||||
#[ts(optional)]
|
||||
pub default_prompt: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -1282,12 +1305,26 @@ impl From<CoreSkillMetadata> for SkillMetadata {
|
||||
name: value.name,
|
||||
description: value.description,
|
||||
short_description: value.short_description,
|
||||
interface: value.interface.map(SkillInterface::from),
|
||||
path: value.path,
|
||||
scope: value.scope.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<CoreSkillInterface> for SkillInterface {
|
||||
fn from(value: CoreSkillInterface) -> Self {
|
||||
Self {
|
||||
display_name: value.display_name,
|
||||
short_description: value.short_description,
|
||||
brand_color: value.brand_color,
|
||||
default_prompt: value.default_prompt,
|
||||
icon_small: value.icon_small,
|
||||
icon_large: value.icon_large,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<CoreSkillScope> for SkillScope {
|
||||
fn from(value: CoreSkillScope) -> Self {
|
||||
match value {
|
||||
|
||||
@@ -3894,6 +3894,16 @@ fn skills_to_info(
|
||||
name: skill.name.clone(),
|
||||
description: skill.description.clone(),
|
||||
short_description: skill.short_description.clone(),
|
||||
interface: skill.interface.clone().map(|interface| {
|
||||
codex_app_server_protocol::SkillInterface {
|
||||
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,
|
||||
}
|
||||
}),
|
||||
path: skill.path.clone(),
|
||||
scope: skill.scope.into(),
|
||||
})
|
||||
|
||||
@@ -33,6 +33,7 @@ use codex_otel::OtelManager;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
@@ -66,6 +67,8 @@ use crate::model_provider_info::WireApi;
|
||||
use crate::tools::spec::create_tools_json_for_chat_completions_api;
|
||||
use crate::tools::spec::create_tools_json_for_responses_api;
|
||||
|
||||
pub const WEB_SEARCH_ELIGIBLE_HEADER: &str = "x-oai-web-search-eligible";
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ModelClientState {
|
||||
config: Arc<Config>,
|
||||
@@ -345,7 +348,7 @@ impl ModelClientSession {
|
||||
store_override: None,
|
||||
conversation_id: Some(conversation_id),
|
||||
session_source: Some(self.state.session_source.clone()),
|
||||
extra_headers,
|
||||
extra_headers: build_responses_headers(&self.state.config),
|
||||
compression,
|
||||
}
|
||||
}
|
||||
@@ -692,6 +695,21 @@ fn beta_feature_headers(config: &Config) -> ApiHeaderMap {
|
||||
headers
|
||||
}
|
||||
|
||||
fn build_responses_headers(config: &Config) -> ApiHeaderMap {
|
||||
let mut headers = beta_feature_headers(config);
|
||||
headers.insert(
|
||||
WEB_SEARCH_ELIGIBLE_HEADER,
|
||||
HeaderValue::from_static(
|
||||
if matches!(config.web_search_mode, Some(WebSearchMode::Disabled)) {
|
||||
"false"
|
||||
} else {
|
||||
"true"
|
||||
},
|
||||
),
|
||||
);
|
||||
headers
|
||||
}
|
||||
|
||||
fn map_response_stream<S>(api_stream: S, otel_manager: OtelManager) -> ResponseStream
|
||||
where
|
||||
S: futures::Stream<Item = std::result::Result<ResponseEvent, ApiError>>
|
||||
|
||||
@@ -120,6 +120,7 @@ use crate::protocol::ReviewDecision;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::protocol::SessionConfiguredEvent;
|
||||
use crate::protocol::SkillErrorInfo;
|
||||
use crate::protocol::SkillInterface as ProtocolSkillInterface;
|
||||
use crate::protocol::SkillMetadata as ProtocolSkillMetadata;
|
||||
use crate::protocol::StreamErrorEvent;
|
||||
use crate::protocol::Submission;
|
||||
@@ -2421,7 +2422,7 @@ async fn spawn_review_thread(
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &review_model_info,
|
||||
features: &review_features,
|
||||
web_search_mode: review_web_search_mode,
|
||||
web_search_mode: Some(review_web_search_mode),
|
||||
});
|
||||
|
||||
let base_instructions = REVIEW_PROMPT.to_string();
|
||||
@@ -2434,7 +2435,7 @@ async fn spawn_review_thread(
|
||||
let mut per_turn_config = (*config).clone();
|
||||
per_turn_config.model = Some(model.clone());
|
||||
per_turn_config.features = review_features.clone();
|
||||
per_turn_config.web_search_mode = review_web_search_mode;
|
||||
per_turn_config.web_search_mode = Some(review_web_search_mode);
|
||||
|
||||
let otel_manager = parent_turn_context
|
||||
.client
|
||||
@@ -2498,6 +2499,17 @@ fn skills_to_info(skills: &[SkillMetadata]) -> Vec<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,
|
||||
}),
|
||||
path: skill.path.clone(),
|
||||
scope: skill.scope,
|
||||
})
|
||||
@@ -2974,7 +2986,6 @@ async fn try_run_turn(
|
||||
should_emit_turn_diff = true;
|
||||
|
||||
needs_follow_up |= sess.has_pending_input().await;
|
||||
error!("needs_follow_up: {needs_follow_up}");
|
||||
|
||||
break Ok(TurnRunResult {
|
||||
needs_follow_up,
|
||||
|
||||
@@ -339,7 +339,8 @@ pub struct Config {
|
||||
/// model info's default preference.
|
||||
pub include_apply_patch_tool: bool,
|
||||
|
||||
pub web_search_mode: WebSearchMode,
|
||||
/// Explicit or feature-derived web search mode.
|
||||
pub web_search_mode: Option<WebSearchMode>,
|
||||
|
||||
/// If set to `true`, used only the experimental unified exec tool.
|
||||
pub use_experimental_unified_exec_tool: bool,
|
||||
@@ -1191,24 +1192,22 @@ pub fn resolve_oss_provider(
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the web search mode from the config, profile, and features.
|
||||
/// Resolve the web search mode from explicit config and feature flags.
|
||||
fn resolve_web_search_mode(
|
||||
config_toml: &ConfigToml,
|
||||
config_profile: &ConfigProfile,
|
||||
features: &Features,
|
||||
) -> WebSearchMode {
|
||||
// Enum gets precedence over features flags
|
||||
) -> Option<WebSearchMode> {
|
||||
if let Some(mode) = config_profile.web_search.or(config_toml.web_search) {
|
||||
return mode;
|
||||
return Some(mode);
|
||||
}
|
||||
if features.enabled(Feature::WebSearchCached) {
|
||||
return WebSearchMode::Cached;
|
||||
return Some(WebSearchMode::Cached);
|
||||
}
|
||||
if features.enabled(Feature::WebSearchRequest) {
|
||||
return WebSearchMode::Live;
|
||||
return Some(WebSearchMode::Live);
|
||||
}
|
||||
// Fall back to default
|
||||
WebSearchMode::default()
|
||||
None
|
||||
}
|
||||
|
||||
impl Config {
|
||||
@@ -2232,15 +2231,12 @@ trust_level = "trusted"
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn web_search_mode_uses_default_if_unset() {
|
||||
fn web_search_mode_uses_none_if_unset() {
|
||||
let cfg = ConfigToml::default();
|
||||
let profile = ConfigProfile::default();
|
||||
let features = Features::with_defaults();
|
||||
|
||||
assert_eq!(
|
||||
resolve_web_search_mode(&cfg, &profile, &features),
|
||||
WebSearchMode::default()
|
||||
);
|
||||
assert_eq!(resolve_web_search_mode(&cfg, &profile, &features), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2255,7 +2251,7 @@ trust_level = "trusted"
|
||||
|
||||
assert_eq!(
|
||||
resolve_web_search_mode(&cfg, &profile, &features),
|
||||
WebSearchMode::Live
|
||||
Some(WebSearchMode::Live)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2271,7 +2267,7 @@ trust_level = "trusted"
|
||||
|
||||
assert_eq!(
|
||||
resolve_web_search_mode(&cfg, &profile, &features),
|
||||
WebSearchMode::Disabled
|
||||
Some(WebSearchMode::Disabled)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3623,7 +3619,7 @@ model_verbosity = "high"
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: false,
|
||||
web_search_mode: WebSearchMode::default(),
|
||||
web_search_mode: None,
|
||||
use_experimental_unified_exec_tool: false,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
@@ -3710,7 +3706,7 @@ model_verbosity = "high"
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: false,
|
||||
web_search_mode: WebSearchMode::default(),
|
||||
web_search_mode: None,
|
||||
use_experimental_unified_exec_tool: false,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
@@ -3812,7 +3808,7 @@ model_verbosity = "high"
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: false,
|
||||
web_search_mode: WebSearchMode::default(),
|
||||
web_search_mode: None,
|
||||
use_experimental_unified_exec_tool: false,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
@@ -3900,7 +3896,7 @@ model_verbosity = "high"
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: false,
|
||||
web_search_mode: WebSearchMode::default(),
|
||||
web_search_mode: None,
|
||||
use_experimental_unified_exec_tool: false,
|
||||
ghost_snapshot: GhostSnapshotConfig::default(),
|
||||
features: Features::with_defaults(),
|
||||
|
||||
@@ -111,6 +111,7 @@ mod user_shell_command;
|
||||
pub mod util;
|
||||
|
||||
pub use apply_patch::CODEX_APPLY_PATCH_ARG1;
|
||||
pub use client::WEB_SEARCH_ELIGIBLE_HEADER;
|
||||
pub use command_safety::is_dangerous_command;
|
||||
pub use command_safety::is_safe_command;
|
||||
pub use exec_policy::ExecPolicyError;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use crate::config::Config;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::skills::model::SkillError;
|
||||
use crate::skills::model::SkillInterface;
|
||||
use crate::skills::model::SkillLoadOutcome;
|
||||
use crate::skills::model::SkillMetadata;
|
||||
use crate::skills::system::system_cache_root_dir;
|
||||
@@ -13,6 +14,7 @@ use std::collections::VecDeque;
|
||||
use std::error::Error;
|
||||
use std::fmt;
|
||||
use std::fs;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tracing::error;
|
||||
@@ -31,11 +33,29 @@ struct SkillFrontmatterMetadata {
|
||||
short_description: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
struct SkillToml {
|
||||
#[serde(default)]
|
||||
interface: Option<Interface>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
struct Interface {
|
||||
display_name: Option<String>,
|
||||
short_description: Option<String>,
|
||||
icon_small: Option<PathBuf>,
|
||||
icon_large: Option<PathBuf>,
|
||||
brand_color: Option<String>,
|
||||
default_prompt: Option<String>,
|
||||
}
|
||||
|
||||
const SKILLS_FILENAME: &str = "SKILL.md";
|
||||
const SKILLS_TOML_FILENAME: &str = "SKILL.toml";
|
||||
const SKILLS_DIR_NAME: &str = "skills";
|
||||
const MAX_NAME_LEN: usize = 64;
|
||||
const MAX_DESCRIPTION_LEN: usize = 1024;
|
||||
const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
const MAX_DEFAULT_PROMPT_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
// Traversal depth from the skills root.
|
||||
const MAX_SCAN_DEPTH: usize = 6;
|
||||
const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000;
|
||||
@@ -195,7 +215,7 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
}
|
||||
}
|
||||
|
||||
// Follow symlinks for user, admin, and repo skills. System skills are written by Codex itself.
|
||||
// Follow symlinked directories for user, admin, and repo skills. System skills are written by Codex itself.
|
||||
let follow_symlinks = matches!(
|
||||
scope,
|
||||
SkillScope::Repo | SkillScope::User | SkillScope::Admin
|
||||
@@ -262,20 +282,6 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
continue;
|
||||
}
|
||||
|
||||
if metadata.is_file() && file_name == SKILLS_FILENAME {
|
||||
match parse_skill_file(&path, scope) {
|
||||
Ok(skill) => outcome.skills.push(skill),
|
||||
Err(err) => {
|
||||
if scope != SkillScope::System {
|
||||
outcome.errors.push(SkillError {
|
||||
path,
|
||||
message: err.to_string(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -336,11 +342,12 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
.as_deref()
|
||||
.map(sanitize_single_line)
|
||||
.filter(|value| !value.is_empty());
|
||||
let interface = load_skill_interface(path);
|
||||
|
||||
validate_field(&name, MAX_NAME_LEN, "name")?;
|
||||
validate_field(&description, MAX_DESCRIPTION_LEN, "description")?;
|
||||
validate_len(&name, MAX_NAME_LEN, "name")?;
|
||||
validate_len(&description, MAX_DESCRIPTION_LEN, "description")?;
|
||||
if let Some(short_description) = short_description.as_deref() {
|
||||
validate_field(
|
||||
validate_len(
|
||||
short_description,
|
||||
MAX_SHORT_DESCRIPTION_LEN,
|
||||
"metadata.short-description",
|
||||
@@ -353,16 +360,115 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
name,
|
||||
description,
|
||||
short_description,
|
||||
interface,
|
||||
path: resolved_path,
|
||||
scope,
|
||||
})
|
||||
}
|
||||
|
||||
fn load_skill_interface(skill_path: &Path) -> Option<SkillInterface> {
|
||||
// Fail open: optional SKILL.toml metadata should not block loading SKILL.md.
|
||||
let skill_dir = skill_path.parent()?;
|
||||
let interface_path = skill_dir.join(SKILLS_TOML_FILENAME);
|
||||
if !interface_path.exists() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let contents = match fs::read_to_string(&interface_path) {
|
||||
Ok(contents) => contents,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
"ignoring {path}: failed to read SKILL.toml: {error}",
|
||||
path = interface_path.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let parsed: SkillToml = match toml::from_str(&contents) {
|
||||
Ok(parsed) => parsed,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
"ignoring {path}: invalid TOML: {error}",
|
||||
path = interface_path.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let interface = parsed.interface?;
|
||||
|
||||
let interface = SkillInterface {
|
||||
display_name: resolve_str(
|
||||
interface.display_name,
|
||||
MAX_NAME_LEN,
|
||||
"interface.display_name",
|
||||
),
|
||||
short_description: resolve_str(
|
||||
interface.short_description,
|
||||
MAX_SHORT_DESCRIPTION_LEN,
|
||||
"interface.short_description",
|
||||
),
|
||||
icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small),
|
||||
icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large),
|
||||
brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"),
|
||||
default_prompt: resolve_str(
|
||||
interface.default_prompt,
|
||||
MAX_DEFAULT_PROMPT_LEN,
|
||||
"interface.default_prompt",
|
||||
),
|
||||
};
|
||||
let has_fields = interface.display_name.is_some()
|
||||
|| interface.short_description.is_some()
|
||||
|| interface.icon_small.is_some()
|
||||
|| interface.icon_large.is_some()
|
||||
|| interface.brand_color.is_some()
|
||||
|| interface.default_prompt.is_some();
|
||||
if has_fields { Some(interface) } else { None }
|
||||
}
|
||||
|
||||
fn resolve_asset_path(
|
||||
skill_dir: &Path,
|
||||
field: &'static str,
|
||||
path: Option<PathBuf>,
|
||||
) -> Option<PathBuf> {
|
||||
// Icons must be relative paths under the skill's assets/ directory; otherwise return None.
|
||||
let path = path?;
|
||||
if path.as_os_str().is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let assets_dir = skill_dir.join("assets");
|
||||
if path.is_absolute() {
|
||||
tracing::warn!(
|
||||
"ignoring {field}: icon must be a relative assets path (not {})",
|
||||
assets_dir.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut components = path.components().peekable();
|
||||
while matches!(components.peek(), Some(Component::CurDir)) {
|
||||
components.next();
|
||||
}
|
||||
match components.next() {
|
||||
Some(Component::Normal(component)) if component == "assets" => {}
|
||||
_ => {
|
||||
tracing::warn!("ignoring {field}: icon path must be under assets/");
|
||||
return None;
|
||||
}
|
||||
}
|
||||
if components.any(|component| matches!(component, Component::ParentDir)) {
|
||||
tracing::warn!("ignoring {field}: icon path must not contain '..'");
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(skill_dir.join(path))
|
||||
}
|
||||
|
||||
fn sanitize_single_line(raw: &str) -> String {
|
||||
raw.split_whitespace().collect::<Vec<_>>().join(" ")
|
||||
}
|
||||
|
||||
fn validate_field(
|
||||
fn validate_len(
|
||||
value: &str,
|
||||
max_len: usize,
|
||||
field_name: &'static str,
|
||||
@@ -379,6 +485,36 @@ fn validate_field(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn resolve_str(value: Option<String>, max_len: usize, field: &'static str) -> Option<String> {
|
||||
let value = value?;
|
||||
let value = sanitize_single_line(&value);
|
||||
if value.is_empty() {
|
||||
tracing::warn!("ignoring {field}: value is empty");
|
||||
return None;
|
||||
}
|
||||
if value.chars().count() > max_len {
|
||||
tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters");
|
||||
return None;
|
||||
}
|
||||
Some(value)
|
||||
}
|
||||
|
||||
fn resolve_color_str(value: Option<String>, field: &'static str) -> Option<String> {
|
||||
let value = value?;
|
||||
let value = value.trim();
|
||||
if value.is_empty() {
|
||||
tracing::warn!("ignoring {field}: value is empty");
|
||||
return None;
|
||||
}
|
||||
let mut chars = value.chars();
|
||||
if value.len() == 7 && chars.next() == Some('#') && chars.all(|c| c.is_ascii_hexdigit()) {
|
||||
Some(value.to_string())
|
||||
} else {
|
||||
tracing::warn!("ignoring {field}: expected #RRGGBB, got {value}");
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn extract_frontmatter(contents: &str) -> Option<String> {
|
||||
let mut lines = contents.lines();
|
||||
if !matches!(lines.next(), Some(line) if line.trim() == "---") {
|
||||
@@ -528,6 +664,224 @@ mod tests {
|
||||
path
|
||||
}
|
||||
|
||||
fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf {
|
||||
let path = skill_dir.join(SKILLS_TOML_FILENAME);
|
||||
fs::write(&path, contents).unwrap();
|
||||
path
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_skill_interface_metadata_happy_path() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml");
|
||||
let skill_dir = skill_path.parent().expect("skill dir");
|
||||
let normalized_skill_dir = normalized(skill_dir);
|
||||
|
||||
write_skill_interface_at(
|
||||
skill_dir,
|
||||
r##"
|
||||
[interface]
|
||||
display_name = "UI Skill"
|
||||
short_description = " short desc "
|
||||
icon_small = "./assets/small-400px.png"
|
||||
icon_large = "./assets/large-logo.svg"
|
||||
brand_color = "#3B82F6"
|
||||
default_prompt = " default prompt "
|
||||
"##,
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "ui-skill".to_string(),
|
||||
description: "from toml".to_string(),
|
||||
short_description: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: Some("UI Skill".to_string()),
|
||||
short_description: Some("short desc".to_string()),
|
||||
icon_small: Some(normalized_skill_dir.join("./assets/small-400px.png")),
|
||||
icon_large: Some(normalized_skill_dir.join("./assets/large-logo.svg")),
|
||||
brand_color: Some("#3B82F6".to_string()),
|
||||
default_prompt: Some("default prompt".to_string()),
|
||||
}),
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn accepts_icon_paths_under_assets_dir() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml");
|
||||
let skill_dir = skill_path.parent().expect("skill dir");
|
||||
let normalized_skill_dir = normalized(skill_dir);
|
||||
|
||||
write_skill_interface_at(
|
||||
skill_dir,
|
||||
r#"
|
||||
[interface]
|
||||
display_name = "UI Skill"
|
||||
icon_small = "assets/icon.png"
|
||||
icon_large = "./assets/logo.svg"
|
||||
"#,
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "ui-skill".to_string(),
|
||||
description: "from toml".to_string(),
|
||||
short_description: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: Some("UI Skill".to_string()),
|
||||
short_description: None,
|
||||
icon_small: Some(normalized_skill_dir.join("assets/icon.png")),
|
||||
icon_large: Some(normalized_skill_dir.join("./assets/logo.svg")),
|
||||
brand_color: None,
|
||||
default_prompt: None,
|
||||
}),
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ignores_invalid_brand_color() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml");
|
||||
let skill_dir = skill_path.parent().expect("skill dir");
|
||||
|
||||
write_skill_interface_at(
|
||||
skill_dir,
|
||||
r#"
|
||||
[interface]
|
||||
brand_color = "blue"
|
||||
"#,
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "ui-skill".to_string(),
|
||||
description: "from toml".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ignores_default_prompt_over_max_length() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml");
|
||||
let skill_dir = skill_path.parent().expect("skill dir");
|
||||
let normalized_skill_dir = normalized(skill_dir);
|
||||
let too_long = "x".repeat(MAX_DEFAULT_PROMPT_LEN + 1);
|
||||
|
||||
write_skill_interface_at(
|
||||
skill_dir,
|
||||
&format!(
|
||||
r##"
|
||||
[interface]
|
||||
display_name = "UI Skill"
|
||||
icon_small = "./assets/small-400px.png"
|
||||
default_prompt = "{too_long}"
|
||||
"##
|
||||
),
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "ui-skill".to_string(),
|
||||
description: "from toml".to_string(),
|
||||
short_description: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: Some("UI Skill".to_string()),
|
||||
short_description: None,
|
||||
icon_small: Some(normalized_skill_dir.join("./assets/small-400px.png")),
|
||||
icon_large: None,
|
||||
brand_color: None,
|
||||
default_prompt: None,
|
||||
}),
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn drops_interface_when_icons_are_invalid() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml");
|
||||
let skill_dir = skill_path.parent().expect("skill dir");
|
||||
|
||||
write_skill_interface_at(
|
||||
skill_dir,
|
||||
r#"
|
||||
[interface]
|
||||
icon_small = "icon.png"
|
||||
icon_large = "./assets/../logo.svg"
|
||||
"#,
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "ui-skill".to_string(),
|
||||
description: "from toml".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn symlink_dir(target: &Path, link: &Path) {
|
||||
std::os::unix::fs::symlink(target, link).unwrap();
|
||||
@@ -563,6 +917,7 @@ mod tests {
|
||||
name: "linked-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&shared_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -571,7 +926,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn loads_skills_via_symlinked_skill_file_for_user_scope() {
|
||||
async fn ignores_symlinked_skill_file_for_user_scope() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let shared = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -590,16 +945,7 @@ mod tests {
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "linked-file-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&shared_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
assert_eq!(outcome.skills, Vec::new());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -629,6 +975,7 @@ mod tests {
|
||||
name: "cycle-skill".to_string(),
|
||||
description: "still loads".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -662,6 +1009,7 @@ mod tests {
|
||||
name: "admin-linked-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&shared_skill_path),
|
||||
scope: SkillScope::Admin,
|
||||
}]
|
||||
@@ -699,6 +1047,7 @@ mod tests {
|
||||
name: "repo-linked-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&linked_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -759,6 +1108,7 @@ mod tests {
|
||||
name: "within-depth-skill".to_string(),
|
||||
description: "loads".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&within_depth_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -783,6 +1133,7 @@ mod tests {
|
||||
name: "demo-skill".to_string(),
|
||||
description: "does things carefully".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -811,6 +1162,7 @@ mod tests {
|
||||
name: "demo-skill".to_string(),
|
||||
description: "long description".to_string(),
|
||||
short_description: Some("short summary".to_string()),
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -920,6 +1272,7 @@ mod tests {
|
||||
name: "repo-skill".to_string(),
|
||||
description: "from repo".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -970,6 +1323,7 @@ mod tests {
|
||||
name: "nested-skill".to_string(),
|
||||
description: "from nested".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&nested_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -977,6 +1331,7 @@ mod tests {
|
||||
name: "root-skill".to_string(),
|
||||
description: "from root".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&root_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
},
|
||||
@@ -1013,6 +1368,7 @@ mod tests {
|
||||
name: "local-skill".to_string(),
|
||||
description: "from cwd".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1050,6 +1406,7 @@ mod tests {
|
||||
name: "dupe-skill".to_string(),
|
||||
description: "from repo".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&repo_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1077,6 +1434,7 @@ mod tests {
|
||||
name: "dupe-skill".to_string(),
|
||||
description: "from user".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&user_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1145,6 +1503,7 @@ mod tests {
|
||||
name: "repo-skill".to_string(),
|
||||
description: "from repo".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1200,6 +1559,7 @@ mod tests {
|
||||
name: "system-skill".to_string(),
|
||||
description: "from system".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::System,
|
||||
}]
|
||||
@@ -1254,6 +1614,7 @@ mod tests {
|
||||
name: "dupe-skill".to_string(),
|
||||
description: "from system".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&system_skill_path),
|
||||
scope: SkillScope::System,
|
||||
}]
|
||||
@@ -1283,6 +1644,7 @@ mod tests {
|
||||
name: "dupe-skill".to_string(),
|
||||
description: "from user".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&user_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
@@ -1321,6 +1683,7 @@ mod tests {
|
||||
name: "dupe-skill".to_string(),
|
||||
description: "from repo".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: normalized(&repo_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
@@ -1371,6 +1734,7 @@ mod tests {
|
||||
name: "dupe-skill".to_string(),
|
||||
description: "from nested".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: expected_path,
|
||||
scope: SkillScope::Repo,
|
||||
}],
|
||||
|
||||
@@ -7,10 +7,21 @@ pub struct SkillMetadata {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
pub short_description: Option<String>,
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub path: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SkillInterface {
|
||||
pub display_name: Option<String>,
|
||||
pub short_description: Option<String>,
|
||||
pub icon_small: Option<PathBuf>,
|
||||
pub icon_large: Option<PathBuf>,
|
||||
pub brand_color: Option<String>,
|
||||
pub default_prompt: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SkillError {
|
||||
pub path: PathBuf,
|
||||
|
||||
@@ -86,7 +86,7 @@ async fn start_review_conversation(
|
||||
let mut sub_agent_config = config.as_ref().clone();
|
||||
// Carry over review-only feature restrictions so the delegate cannot
|
||||
// re-enable blocked tools (web search, view image).
|
||||
sub_agent_config.web_search_mode = WebSearchMode::Disabled;
|
||||
sub_agent_config.web_search_mode = Some(WebSearchMode::Disabled);
|
||||
|
||||
// Set explicit review rubric for the sub-agent
|
||||
sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string());
|
||||
|
||||
@@ -25,7 +25,7 @@ use std::collections::HashMap;
|
||||
pub(crate) struct ToolsConfig {
|
||||
pub shell_type: ConfigShellToolType,
|
||||
pub apply_patch_tool_type: Option<ApplyPatchToolType>,
|
||||
pub web_search_mode: WebSearchMode,
|
||||
pub web_search_mode: Option<WebSearchMode>,
|
||||
pub collab_tools: bool,
|
||||
pub experimental_supported_tools: Vec<String>,
|
||||
}
|
||||
@@ -33,7 +33,7 @@ pub(crate) struct ToolsConfig {
|
||||
pub(crate) struct ToolsConfigParams<'a> {
|
||||
pub(crate) model_info: &'a ModelInfo,
|
||||
pub(crate) features: &'a Features,
|
||||
pub(crate) web_search_mode: WebSearchMode,
|
||||
pub(crate) web_search_mode: Option<WebSearchMode>,
|
||||
}
|
||||
|
||||
impl ToolsConfig {
|
||||
@@ -1244,17 +1244,17 @@ pub(crate) fn build_specs(
|
||||
}
|
||||
|
||||
match config.web_search_mode {
|
||||
WebSearchMode::Disabled => {}
|
||||
WebSearchMode::Cached => {
|
||||
Some(WebSearchMode::Cached) => {
|
||||
builder.push_spec(ToolSpec::WebSearch {
|
||||
external_web_access: Some(false),
|
||||
});
|
||||
}
|
||||
WebSearchMode::Live => {
|
||||
Some(WebSearchMode::Live) => {
|
||||
builder.push_spec(ToolSpec::WebSearch {
|
||||
external_web_access: Some(true),
|
||||
});
|
||||
}
|
||||
Some(WebSearchMode::Disabled) | None => {}
|
||||
}
|
||||
|
||||
builder.push_spec_with_parallel_support(create_view_image_tool(), true);
|
||||
@@ -1398,7 +1398,7 @@ mod tests {
|
||||
let config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Live,
|
||||
web_search_mode: Some(WebSearchMode::Live),
|
||||
});
|
||||
let (tools, _) = build_specs(&config, None).build();
|
||||
|
||||
@@ -1460,7 +1460,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, None).build();
|
||||
assert_contains_tool_names(
|
||||
@@ -1472,7 +1472,7 @@ mod tests {
|
||||
fn assert_model_tools(
|
||||
model_slug: &str,
|
||||
features: &Features,
|
||||
web_search_mode: WebSearchMode,
|
||||
web_search_mode: Option<WebSearchMode>,
|
||||
expected_tools: &[&str],
|
||||
) {
|
||||
let config = test_config();
|
||||
@@ -1496,7 +1496,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, None).build();
|
||||
|
||||
@@ -1518,7 +1518,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Live,
|
||||
web_search_mode: Some(WebSearchMode::Live),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, None).build();
|
||||
|
||||
@@ -1536,7 +1536,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5-codex",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"shell_command",
|
||||
"list_mcp_resources",
|
||||
@@ -1555,7 +1555,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5.1-codex",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"shell_command",
|
||||
"list_mcp_resources",
|
||||
@@ -1574,7 +1574,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5-codex",
|
||||
Features::with_defaults().enable(Feature::UnifiedExec),
|
||||
WebSearchMode::Live,
|
||||
Some(WebSearchMode::Live),
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
@@ -1594,7 +1594,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5.1-codex",
|
||||
Features::with_defaults().enable(Feature::UnifiedExec),
|
||||
WebSearchMode::Live,
|
||||
Some(WebSearchMode::Live),
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
@@ -1614,7 +1614,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"codex-mini-latest",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"local_shell",
|
||||
"list_mcp_resources",
|
||||
@@ -1632,7 +1632,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5.1-codex-mini",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"shell_command",
|
||||
"list_mcp_resources",
|
||||
@@ -1651,7 +1651,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"shell",
|
||||
"list_mcp_resources",
|
||||
@@ -1669,7 +1669,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"gpt-5.1",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"shell_command",
|
||||
"list_mcp_resources",
|
||||
@@ -1688,7 +1688,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"exp-5.1",
|
||||
&Features::with_defaults(),
|
||||
WebSearchMode::Cached,
|
||||
Some(WebSearchMode::Cached),
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
@@ -1708,7 +1708,7 @@ mod tests {
|
||||
assert_model_tools(
|
||||
"codex-mini-latest",
|
||||
Features::with_defaults().enable(Feature::UnifiedExec),
|
||||
WebSearchMode::Live,
|
||||
Some(WebSearchMode::Live),
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
@@ -1731,7 +1731,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Live,
|
||||
web_search_mode: Some(WebSearchMode::Live),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, Some(HashMap::new())).build();
|
||||
|
||||
@@ -1753,7 +1753,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, None).build();
|
||||
|
||||
@@ -1772,7 +1772,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, None).build();
|
||||
|
||||
@@ -1803,7 +1803,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Live,
|
||||
web_search_mode: Some(WebSearchMode::Live),
|
||||
});
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
@@ -1898,7 +1898,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
|
||||
// Intentionally construct a map with keys that would sort alphabetically.
|
||||
@@ -1975,7 +1975,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
|
||||
let (tools, _) = build_specs(
|
||||
@@ -2032,7 +2032,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
|
||||
let (tools, _) = build_specs(
|
||||
@@ -2086,7 +2086,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
|
||||
let (tools, _) = build_specs(
|
||||
@@ -2142,7 +2142,7 @@ mod tests {
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
|
||||
let (tools, _) = build_specs(
|
||||
@@ -2254,7 +2254,7 @@ Examples of valid command strings:
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: WebSearchMode::Cached,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
|
||||
@@ -9,15 +9,18 @@ use codex_core::ModelProviderInfo;
|
||||
use codex_core::Prompt;
|
||||
use codex_core::ResponseEvent;
|
||||
use codex_core::ResponseItem;
|
||||
use codex_core::WEB_SEARCH_ELIGIBLE_HEADER;
|
||||
use codex_core::WireApi;
|
||||
use codex_core::models_manager::manager::ModelsManager;
|
||||
use codex_otel::OtelManager;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use futures::StreamExt;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::matchers::header;
|
||||
@@ -213,6 +216,66 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn responses_stream_includes_web_search_eligible_header_true_by_default() {
|
||||
core_test_support::skip_if_no_network!();
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response_body = responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]);
|
||||
|
||||
let request_recorder = responses::mount_sse_once_match(
|
||||
&server,
|
||||
header(WEB_SEARCH_ELIGIBLE_HEADER, "true"),
|
||||
response_body,
|
||||
)
|
||||
.await;
|
||||
|
||||
let test = test_codex().build(&server).await.expect("build test codex");
|
||||
test.submit_turn("hello").await.expect("submit test prompt");
|
||||
|
||||
let request = request_recorder.single_request();
|
||||
assert_eq!(
|
||||
request.header(WEB_SEARCH_ELIGIBLE_HEADER).as_deref(),
|
||||
Some("true")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn responses_stream_includes_web_search_eligible_header_false_when_disabled() {
|
||||
core_test_support::skip_if_no_network!();
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response_body = responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]);
|
||||
|
||||
let request_recorder = responses::mount_sse_once_match(
|
||||
&server,
|
||||
header(WEB_SEARCH_ELIGIBLE_HEADER, "false"),
|
||||
response_body,
|
||||
)
|
||||
.await;
|
||||
|
||||
let test = test_codex()
|
||||
.with_config(|config| {
|
||||
config.web_search_mode = Some(WebSearchMode::Disabled);
|
||||
})
|
||||
.build(&server)
|
||||
.await
|
||||
.expect("build test codex");
|
||||
test.submit_turn("hello").await.expect("submit test prompt");
|
||||
|
||||
let request = request_recorder.single_request();
|
||||
assert_eq!(
|
||||
request.header(WEB_SEARCH_ELIGIBLE_HEADER).as_deref(),
|
||||
Some("false")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn responses_respects_model_info_overrides_from_config() {
|
||||
core_test_support::skip_if_no_network!();
|
||||
|
||||
@@ -14,6 +14,7 @@ use codex_core::ThreadManager;
|
||||
use codex_core::WireApi;
|
||||
use codex_core::auth::AuthCredentialsStoreMode;
|
||||
use codex_core::built_in_model_providers;
|
||||
use codex_core::default_client::originator;
|
||||
use codex_core::error::CodexErr;
|
||||
use codex_core::models_manager::manager::ModelsManager;
|
||||
use codex_core::protocol::EventMsg;
|
||||
@@ -406,7 +407,7 @@ async fn includes_conversation_id_and_model_headers_in_request() {
|
||||
let request_originator = request.header("originator").expect("originator header");
|
||||
|
||||
assert_eq!(request_session_id, session_id.to_string());
|
||||
assert_eq!(request_originator, "codex_cli_rs");
|
||||
assert_eq!(request_originator, originator().value);
|
||||
assert_eq!(request_authorization, "Bearer Test API Key");
|
||||
}
|
||||
|
||||
@@ -522,7 +523,7 @@ async fn chatgpt_auth_sends_correct_request() {
|
||||
let session_id = request.header("session_id").expect("session_id header");
|
||||
assert_eq!(session_id, thread_id.to_string());
|
||||
|
||||
assert_eq!(request_originator, "codex_cli_rs");
|
||||
assert_eq!(request_originator, originator().value);
|
||||
assert_eq!(request_authorization, "Bearer Access Token");
|
||||
assert_eq!(request_chatgpt_account_id, "account_id");
|
||||
assert!(request_body["stream"].as_bool().unwrap());
|
||||
|
||||
@@ -36,7 +36,7 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec<String> {
|
||||
let mut builder = test_codex()
|
||||
.with_model(model)
|
||||
// Keep tool expectations stable when the default web_search mode changes.
|
||||
.with_config(|config| config.web_search_mode = WebSearchMode::Cached);
|
||||
.with_config(|config| config.web_search_mode = Some(WebSearchMode::Cached));
|
||||
let test = builder
|
||||
.build(&server)
|
||||
.await
|
||||
|
||||
@@ -88,7 +88,7 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> {
|
||||
config.user_instructions = Some("be consistent and helpful".to_string());
|
||||
config.model = Some("gpt-5.1-codex-max".to_string());
|
||||
// Keep tool expectations stable when the default web_search mode changes.
|
||||
config.web_search_mode = WebSearchMode::Cached;
|
||||
config.web_search_mode = Some(WebSearchMode::Cached);
|
||||
})
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
@@ -35,7 +35,7 @@ async fn web_search_mode_cached_sets_external_web_access_false_in_request_body()
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5-codex")
|
||||
.with_config(|config| {
|
||||
config.web_search_mode = WebSearchMode::Cached;
|
||||
config.web_search_mode = Some(WebSearchMode::Cached);
|
||||
});
|
||||
let test = builder
|
||||
.build(&server)
|
||||
@@ -67,7 +67,7 @@ async fn web_search_mode_takes_precedence_over_legacy_flags_in_request_body() {
|
||||
.with_model("gpt-5-codex")
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::WebSearchRequest);
|
||||
config.web_search_mode = WebSearchMode::Cached;
|
||||
config.web_search_mode = Some(WebSearchMode::Cached);
|
||||
});
|
||||
let test = builder
|
||||
.build(&server)
|
||||
|
||||
@@ -1962,13 +1962,33 @@ pub enum SkillScope {
|
||||
pub struct SkillMetadata {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
#[ts(optional)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
/// Legacy short_description from SKILL.md. Prefer SKILL.toml interface.short_description.
|
||||
pub short_description: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub path: PathBuf,
|
||||
pub scope: SkillScope,
|
||||
}
|
||||
|
||||
#[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<PathBuf>,
|
||||
#[ts(optional)]
|
||||
pub icon_large: Option<PathBuf>,
|
||||
#[ts(optional)]
|
||||
pub brand_color: Option<String>,
|
||||
#[ts(optional)]
|
||||
pub default_prompt: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct SkillErrorInfo {
|
||||
pub path: PathBuf,
|
||||
|
||||
@@ -87,12 +87,8 @@ impl SkillPopup {
|
||||
.into_iter()
|
||||
.map(|(idx, indices, _score)| {
|
||||
let skill = &self.skills[idx];
|
||||
let name = truncate_text(&skill.name, 21);
|
||||
let description = skill
|
||||
.short_description
|
||||
.as_ref()
|
||||
.unwrap_or(&skill.description)
|
||||
.clone();
|
||||
let name = truncate_text(skill_display_name(skill), 21);
|
||||
let description = skill_description(skill).to_string();
|
||||
GenericDisplayRow {
|
||||
name,
|
||||
match_indices: indices,
|
||||
@@ -117,15 +113,20 @@ impl SkillPopup {
|
||||
}
|
||||
|
||||
for (idx, skill) in self.skills.iter().enumerate() {
|
||||
if let Some((indices, score)) = fuzzy_match(&skill.name, filter) {
|
||||
let display_name = skill_display_name(skill);
|
||||
if let Some((indices, score)) = fuzzy_match(display_name, filter) {
|
||||
out.push((idx, Some(indices), score));
|
||||
} else if display_name != skill.name
|
||||
&& let Some((_indices, score)) = fuzzy_match(&skill.name, filter)
|
||||
{
|
||||
out.push((idx, None, score));
|
||||
}
|
||||
}
|
||||
|
||||
out.sort_by(|a, b| {
|
||||
a.2.cmp(&b.2).then_with(|| {
|
||||
let an = &self.skills[a.0].name;
|
||||
let bn = &self.skills[b.0].name;
|
||||
let an = skill_display_name(&self.skills[a.0]);
|
||||
let bn = skill_display_name(&self.skills[b.0]);
|
||||
an.cmp(bn)
|
||||
})
|
||||
});
|
||||
@@ -177,3 +178,20 @@ fn skill_popup_hint_line() -> Line<'static> {
|
||||
" to close".into(),
|
||||
])
|
||||
}
|
||||
|
||||
fn skill_display_name(skill: &SkillMetadata) -> &str {
|
||||
skill
|
||||
.interface
|
||||
.as_ref()
|
||||
.and_then(|interface| interface.display_name.as_deref())
|
||||
.unwrap_or(&skill.name)
|
||||
}
|
||||
|
||||
fn skill_description(skill: &SkillMetadata) -> &str {
|
||||
skill
|
||||
.interface
|
||||
.as_ref()
|
||||
.and_then(|interface| interface.short_description.as_deref())
|
||||
.or(skill.short_description.as_deref())
|
||||
.unwrap_or(&skill.description)
|
||||
}
|
||||
|
||||
@@ -87,6 +87,7 @@ use codex_core::protocol::ViewImageToolCallEvent;
|
||||
use codex_core::protocol::WarningEvent;
|
||||
use codex_core::protocol::WebSearchBeginEvent;
|
||||
use codex_core::protocol::WebSearchEndEvent;
|
||||
use codex_core::skills::model::SkillInterface;
|
||||
use codex_core::skills::model::SkillMetadata;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::account::PlanType;
|
||||
@@ -4594,6 +4595,14 @@ fn skills_for_cwd(cwd: &Path, skills_entries: &[SkillsListEntry]) -> Vec<SkillMe
|
||||
name: skill.name.clone(),
|
||||
description: skill.description.clone(),
|
||||
short_description: skill.short_description.clone(),
|
||||
interface: skill.interface.clone().map(|interface| SkillInterface {
|
||||
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,
|
||||
}),
|
||||
path: skill.path.clone(),
|
||||
scope: skill.scope,
|
||||
})
|
||||
|
||||
@@ -87,12 +87,8 @@ impl SkillPopup {
|
||||
.into_iter()
|
||||
.map(|(idx, indices, _score)| {
|
||||
let skill = &self.skills[idx];
|
||||
let name = truncate_text(&skill.name, 21);
|
||||
let description = skill
|
||||
.short_description
|
||||
.as_ref()
|
||||
.unwrap_or(&skill.description)
|
||||
.clone();
|
||||
let name = truncate_text(skill_display_name(skill), 21);
|
||||
let description = skill_description(skill).to_string();
|
||||
GenericDisplayRow {
|
||||
name,
|
||||
match_indices: indices,
|
||||
@@ -116,15 +112,20 @@ impl SkillPopup {
|
||||
}
|
||||
|
||||
for (idx, skill) in self.skills.iter().enumerate() {
|
||||
if let Some((indices, score)) = fuzzy_match(&skill.name, filter) {
|
||||
let display_name = skill_display_name(skill);
|
||||
if let Some((indices, score)) = fuzzy_match(display_name, filter) {
|
||||
out.push((idx, Some(indices), score));
|
||||
} else if display_name != skill.name
|
||||
&& let Some((_indices, score)) = fuzzy_match(&skill.name, filter)
|
||||
{
|
||||
out.push((idx, None, score));
|
||||
}
|
||||
}
|
||||
|
||||
out.sort_by(|a, b| {
|
||||
a.2.cmp(&b.2).then_with(|| {
|
||||
let an = &self.skills[a.0].name;
|
||||
let bn = &self.skills[b.0].name;
|
||||
let an = skill_display_name(&self.skills[a.0]);
|
||||
let bn = skill_display_name(&self.skills[b.0]);
|
||||
an.cmp(bn)
|
||||
})
|
||||
});
|
||||
@@ -176,3 +177,20 @@ fn skill_popup_hint_line() -> Line<'static> {
|
||||
" to close".into(),
|
||||
])
|
||||
}
|
||||
|
||||
fn skill_display_name(skill: &SkillMetadata) -> &str {
|
||||
skill
|
||||
.interface
|
||||
.as_ref()
|
||||
.and_then(|interface| interface.display_name.as_deref())
|
||||
.unwrap_or(&skill.name)
|
||||
}
|
||||
|
||||
fn skill_description(skill: &SkillMetadata) -> &str {
|
||||
skill
|
||||
.interface
|
||||
.as_ref()
|
||||
.and_then(|interface| interface.short_description.as_deref())
|
||||
.or(skill.short_description.as_deref())
|
||||
.unwrap_or(&skill.description)
|
||||
}
|
||||
|
||||
@@ -85,6 +85,7 @@ use codex_core::protocol::ViewImageToolCallEvent;
|
||||
use codex_core::protocol::WarningEvent;
|
||||
use codex_core::protocol::WebSearchBeginEvent;
|
||||
use codex_core::protocol::WebSearchEndEvent;
|
||||
use codex_core::skills::model::SkillInterface;
|
||||
use codex_core::skills::model::SkillMetadata;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::account::PlanType;
|
||||
@@ -4303,6 +4304,14 @@ fn skills_for_cwd(cwd: &Path, skills_entries: &[SkillsListEntry]) -> Vec<SkillMe
|
||||
name: skill.name.clone(),
|
||||
description: skill.description.clone(),
|
||||
short_description: skill.short_description.clone(),
|
||||
interface: skill.interface.clone().map(|interface| SkillInterface {
|
||||
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,
|
||||
}),
|
||||
path: skill.path.clone(),
|
||||
scope: skill.scope,
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user