mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Merge remote-tracking branch 'origin/main' into codex/pr27914-refresh
This commit is contained in:
@@ -651,8 +651,19 @@ async fn parse_skill_file(
|
||||
|
||||
let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?;
|
||||
|
||||
let parsed: SkillFrontmatter =
|
||||
serde_yaml::from_str(&frontmatter).map_err(SkillParseError::InvalidYaml)?;
|
||||
let parsed: SkillFrontmatter = match serde_yaml::from_str(&frontmatter) {
|
||||
Ok(parsed) => Ok(parsed),
|
||||
Err(original_error) => match repair_frontmatter_scalar_fields(&frontmatter) {
|
||||
// Some third-party skills use prose like `description: Build for AWS: ECS`
|
||||
// or `argument-hint: <duration: e.g. 7d>`. Keep the repair line-oriented
|
||||
// so unrelated invalid YAML still surfaces.
|
||||
Some(repaired_frontmatter) => {
|
||||
serde_yaml::from_str(&repaired_frontmatter).map_err(|_| original_error)
|
||||
}
|
||||
None => Err(original_error),
|
||||
},
|
||||
}
|
||||
.map_err(SkillParseError::InvalidYaml)?;
|
||||
|
||||
let base_name = parsed
|
||||
.name
|
||||
@@ -997,6 +1008,91 @@ fn sanitize_single_line(raw: &str) -> String {
|
||||
raw.split_whitespace().collect::<Vec<_>>().join(" ")
|
||||
}
|
||||
|
||||
fn repair_frontmatter_scalar_fields(frontmatter: &str) -> Option<String> {
|
||||
let mut changed = false;
|
||||
let mut block_scalar_indent: Option<usize> = None;
|
||||
let mut repaired_lines: Vec<String> = Vec::new();
|
||||
for line in frontmatter.lines() {
|
||||
let indent = line
|
||||
.chars()
|
||||
.take_while(|character| *character == ' ')
|
||||
.count();
|
||||
if let Some(block_indent) = block_scalar_indent {
|
||||
if line.trim().is_empty() || indent > block_indent {
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
}
|
||||
block_scalar_indent = None;
|
||||
}
|
||||
|
||||
let Some((key, value)) = line.split_once(':') else {
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
};
|
||||
if key.trim().is_empty() || !value.chars().next().is_none_or(char::is_whitespace) {
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
}
|
||||
|
||||
let trimmed_start = value.trim_start();
|
||||
let leading_whitespace = &value[..value.len() - trimmed_start.len()];
|
||||
let mut scalar = trimmed_start;
|
||||
let mut comment = "";
|
||||
for (index, character) in trimmed_start.char_indices() {
|
||||
if character == '#'
|
||||
&& (index == 0
|
||||
|| trimmed_start[..index]
|
||||
.chars()
|
||||
.next_back()
|
||||
.is_some_and(char::is_whitespace))
|
||||
{
|
||||
let comment_start = trimmed_start[..index].trim_end().len();
|
||||
scalar = &trimmed_start[..comment_start];
|
||||
comment = &trimmed_start[comment_start..];
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let scalar = scalar.trim_end();
|
||||
let Some(first_char) = scalar.chars().next() else {
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
};
|
||||
if matches!(first_char, '|' | '>') {
|
||||
block_scalar_indent = Some(indent);
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
}
|
||||
if matches!(first_char, '\'' | '"') {
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
}
|
||||
let mut has_colon_separator = false;
|
||||
let mut chars = scalar.chars().peekable();
|
||||
while let Some(character) = chars.next() {
|
||||
if character == ':'
|
||||
&& matches!(chars.peek(), Some(next_character) if next_character.is_whitespace())
|
||||
{
|
||||
has_colon_separator = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
let invalid_flow_like_scalar = matches!(first_char, '[' | '{' | '@' | '`')
|
||||
&& serde_yaml::from_str::<serde_yaml::Value>(scalar).is_err();
|
||||
if !has_colon_separator && !invalid_flow_like_scalar {
|
||||
repaired_lines.push(line.to_string());
|
||||
continue;
|
||||
}
|
||||
|
||||
let quoted_scalar = format!("'{}'", scalar.replace('\'', "''"));
|
||||
repaired_lines.push(format!(
|
||||
"{key}:{leading_whitespace}{quoted_scalar}{comment}"
|
||||
));
|
||||
changed = true;
|
||||
}
|
||||
changed.then(|| repaired_lines.join("\n"))
|
||||
}
|
||||
|
||||
fn validate_len(
|
||||
value: &str,
|
||||
max_len: usize,
|
||||
|
||||
@@ -1411,6 +1411,134 @@ async fn loads_short_description_from_metadata() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_unquoted_description_containing_colon_space() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_raw_skill_at(
|
||||
&codex_home.path().join("skills"),
|
||||
"colon-description",
|
||||
"name: colon-description\ndescription: AWS deployment patterns: ECS Fargate, Lambda, and S3",
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills_for_test(&cfg).await;
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "colon-description".to_string(),
|
||||
description: "AWS deployment patterns: ECS Fargate, Lambda, and S3".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_unquoted_short_description_containing_colon_space_and_apostrophe() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_raw_skill_at(
|
||||
&codex_home.path().join("skills"),
|
||||
"colon-short-description",
|
||||
"name: colon-short-description\ndescription: long description\nmetadata:\n short-description: What's included: builds and tests",
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills_for_test(&cfg).await;
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "colon-short-description".to_string(),
|
||||
description: "long description".to_string(),
|
||||
short_description: Some("What's included: builds and tests".to_string()),
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_unrecognized_frontmatter_fields_that_need_quotes() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_raw_skill_at(
|
||||
&codex_home.path().join("skills"),
|
||||
"repaired-unknown-fields",
|
||||
"name: repaired-unknown-fields\ndescription: valid description\nargument-hint: <duration: e.g. 7d, 2w>\ntags: [next,@supabase/ssr]",
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills_for_test(&cfg).await;
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "repaired-unknown-fields".to_string(),
|
||||
description: "valid description".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preserves_block_scalar_body_while_repairing_other_fields() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_raw_skill_at(
|
||||
&codex_home.path().join("skills"),
|
||||
"block-description-with-repair",
|
||||
"name: block-description-with-repair\ndescription: |-\n Build for AWS: ECS\nargument-hint: <duration: e.g. 7d>",
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills_for_test(&cfg).await;
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "block-description-with-repair".to_string(),
|
||||
description: "Build for AWS: ECS".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn enforces_short_description_length_limits() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user